• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Ron McLeod
  • Paul Clapham
  • Devaka Cooray
  • Tim Cooke
Sheriffs:
  • Rob Spoor
  • Liutauras Vilda
  • paul wheaton
Saloon Keepers:
  • Tim Holloway
  • Tim Moores
  • Mikalai Zaikin
  • Carey Brown
  • Piet Souris
Bartenders:
  • Stephan van Hulst

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

 
Ranch Hand
Posts: 59
Eclipse IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.



 
Marshal
Posts: 28303
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Bartender
Posts: 1051
5
Hibernate Eclipse IDE Chrome
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
This tiny ad will self destruct in five seconds.
Smokeless wood heat with a rocket mass heater
https://woodheat.net
reply
    Bookmark Topic Watch Topic
  • New Topic