• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

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

 
Kristjan Toots
Ranch Hand
Posts: 59
Eclipse IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.



 
Paul Clapham
Sheriff
Pie
Posts: 20971
31
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 59
Eclipse IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 1051
5
Chrome Eclipse IDE Hibernate
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic