This week's giveaway is in the Android forum.
We're giving away four copies of Android Security Essentials Live Lessons and have Godfrey Nolan on-line!
See this thread for details.
The moose likes Beginning Java and the fly likes Constructive criticism about my code appreciated, with comments 150 lines. Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Android Security Essentials Live Lessons this week in the Android forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Constructive criticism about my code appreciated, with comments 150 lines." Watch "Constructive criticism about my code appreciated, with comments 150 lines." New topic
Author

Constructive criticism about my code appreciated, with comments 150 lines.

Kristjan Toots
Ranch Hand

Joined: Jun 03, 2011
Posts: 59

Hello fellow Ranchers!
I am self-learnt "pre-junior" programmer who hasn't done much except some console practice exercises from Java books. I tend to be quite strict about my code and I definitely do not want to pick up some bad habits along the way.
I like constructive criticism because only then I'm able to learn from my mistakes.
If you have some time, I would welcome you to take a quick look on my code.

Thank you. : )

TASK: I have a string where I get a date in the following format: "Since 21.november 2011" and it applyes for the whole week. Next week it will be: "Since 28.november 2011".
I have to get the week number out of this data.





Please correct my english.
Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

Normally I would say "Don't write any code which somebody else has already written for you". And in a case like this, I would normally suggest using a SimpleDateFormat to parse a pre-defined date format into a Date object, instead of doing all the parsing in your own code like you did. And I see that Java does indeed have an Estonian locale: http://www.localeplanet.com/java/et-EE/index.html. It might not handle the genitive month names that you get in your input, but it might be worth a try anyway.
Kristjan Toots
Ranch Hand

Joined: Jun 03, 2011
Posts: 59

Paul Clapham, thank you for your time and effort.
I don't know why I overlooked SimpleDateFormat, maybe because I was so excited doing something my own instead...

I looked into it, and with only couple of minutes I was able to create the code needed to produce the current date, formatting following the very same signature of genitive form.
I am certain that reversing this step can't be much more difficult.

Since now we know that I tried to re-invent the bicycle and the code i submitted doesn't matter much anymore, still everybody are welcomed to come criticise my code.
James Boswell
Bartender

Joined: Nov 09, 2011
Posts: 1012
    
    5

Just my thoughts, all around the getMonthInt(String) method.

1) Check for null argument and if so, return 0.

2) 0 will be returned for jaanuarist, 1 for veebruarist and so on. Don't you want 1 for jaanuarist, 2 for veebruarist...?

3) The argument "jaanuarist" and "something_not_in_months_array" will return the same result - 0. Is this intended behaviour?

------------------------
4) The instance member matcher should be private.
 
 
subject: Constructive criticism about my code appreciated, with comments 150 lines.
 
Similar Threads
replacing string in java
Date Class Problems
Need to tokenize a String , but i need to keep what comes between "and"
how to test the setter methods
toSrting() method