my dog learned polymorphism*
The moose likes Java in General and the fly likes Please review my code Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


JavaRanch » Java Forums » Java » Java in General
Bookmark "Please review my code" Watch "Please review my code" New topic
Author

Please review my code

miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
critics to my code, solutions, etc would be wellcome!

i want to convert a String to a Date.

the initial format is: dd/mm/yyyy; so regex (copy/pasted from net) works on this, but, as user might go wrong, i check if there are single day/month, for example: 3/5/2005 or 31/4/2004 or 7/12/2000.

here�s my class:


along with that class i've these tests:

Is code clear enough?
Where can i improve code?
Are tests enough?
Are they aceptably conceived?

Any feeedback would be of great help!

TIA


java amateur
Nick George
Ranch Hand

Joined: Apr 04, 2004
Posts: 815
I suggest you try posting here


I've heard it takes forever to grow a woman from the ground
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
i hesitated between posting here or at beguinners forum, but since i dont consider my post a joke i'll not follow your idea
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24187
    
  34

A few random thoughts -- not a thorough review:

Well, even though you got that regex from a web site, it's so hideously long and scary that I'd really never want to see it in actual code.

The first two methods in the above are identical except for two numbers -- you should use just one routine with arguments. If you want the two separate routines because the names are meaningful, factor the duplicate code into a third routine.

It's silly to test "dDate instanceof Date", because if it weren't, you wouldn't have been able to compile the code in the first place (unless it's null, in which case you ought to just test for null, right?)


[Jess in Action][AskingGoodQuestions]
kri shan
Ranch Hand

Joined: Apr 08, 2004
Posts: 1376
Why don't you try thru SimpleDateFormat for converting String to Date ?
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
Ernest,
thank you for your usefull comments.

i actually discovered some inaccuracies (before reading your answer) and declared all vars private, all methods (except converte()) private too and also moved regex declaration to class level;

before i refactored to what i have post, i simply had:

but then this could happen:

which prints:

Tue Apr 02 00:00:00 GMT 20

wanting to avoid this was the main reason for chasing a regex...

What do you suggest as an alternative?

TIA
Jeanne Boyarsky
author & internet detective
Marshal

Joined: May 26, 2003
Posts: 30596
    
154

Miguel,
Take a look at the DateFormat.setLenient() method. It will throw an exception if the user enters an invalid date.


[Blog] [JavaRanch FAQ] [How To Ask Questions The Smart Way] [Book Promos]
Blogging on Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, OCAJP, OCPJP beta, TOGAF part 1 and part 2
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
Jeanne,
thanks for your suggestion

i did take a look:
(class DateFormat)
setLenient
public void setLenient(boolean lenient)Specify whether or not date/time parsing is to be lenient. With lenient parsing, the parser may use heuristics to interpret inputs that do not precisely match this object's format. With strict parsing, inputs must match this object's format.
(class Calendar)
setLenient
public void setLenient(boolean lenient)Specify whether or not date/time interpretation is to be lenient. With lenient interpretation, a date such as "February 942, 1996" will be treated as being equivalent to the 941st day after February 1, 1996. With strict interpretation, such dates will cause an exception to be thrown.

with lenient==true prints:
Tue Mar 01 00:00:00 GMT 2005
else:
Exception in thread "main" java.text.ParseException: Unparseable date: "29/02/2005"
at java.text.DateFormat.parse(DateFormat.java:335)
at DatasUtil.converte1(DatasUtil.java:79)
at DatasUtil.main(DatasUtil.java:84)

but i dont understand:
With lenient parsing, the parser may use heuristics to interpret inputs that do not precisely match this object's format

would that help me? how?

i understand heuristics as a method of one discovering for himself...

TIA
Jeanne Boyarsky
author & internet detective
Marshal

Joined: May 26, 2003
Posts: 30596
    
154

Miguel,
The description of lenient and your experience running it match, which is good.

When lenient is true, the JVM uses rules (heuristics) to guess what you meant. So it figures that 2/29 is the day after 2/28. When lenient is false, the JVM throws an exception (as you saw.)

The definition of heuristics is actually pretty interesting. #3 is the one that applies here. The JVM uses the best of several methods to arrive at what date you meant.
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
Jeanne,
you'r quite kind, thanks a lot!

i feel quite embarrased for keeping asking but,
i now understand that default is lenient==true, but i guess my main prob stands without solution:
"2/4/200p5" will keep passing undetected unless i use regex

TIA
David Harkness
Ranch Hand

Joined: Aug 07, 2003
Posts: 1646
No worries, Miguel. Date parsing isn't exactly a trivial exercise.

If I understand your requirements, you'd like the user to be able to enter a date as 1 or 2 digits for day-of-month, a /, 1 or 2 digits for month, a /, and 4 digits for year. Any other characters should cause the parsing to fail.

To do this, setLenient to false (this will block "20p5" from being treated as year 20 (or 1920 or 2020, whatever it's doing in this case). To allow 1 or 2 digits for month and day-of-month, use the single character "M" and "d":The value "2/4/20p5" will now throw a ParseException.

If this isn't what you're seeking, can you explain in words exactly what you want to have happen when the user enters "2/4/20p5"? Should it magically convert it to "2/4/2005"? If not, what?
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
David,
thanks for helping!,
but


Thu Feb 29 00:00:00 GMT 20

if instead string is "29/2/21a5" then throws exception indeed because isnt a leap year
but again string "2/4/20p5" outputs:

Tue Apr 02 00:00:00 GMT 20

[ April 02, 2005: Message edited by: miguel lisboa ]
David Harkness
Ranch Hand

Joined: Aug 07, 2003
Posts: 1646
Looks like I learned my new thing for today. I assumed parse(String) would throw an exception for any invalid characters, but it seems to stop once the full format has been seen. As well, I was also under the impression that when parsing the number of pattern letters was important. However, it's ignore unless it's ambiguous.

For example, "d/M/y" is equivalent to "dd/MM/yyyy" (both will parse "28/2/20", but "dMy" is not equivalent to "ddMMyyyy" because the parser can't tell what date "11111" represents. The use of separators removes ambiguity.

It seems you have two choices. One is to use regex to validate the full input string before parsing, the other is to use the second form of parse(String, ParsePosition).

For the regex, you can get by with a much simpler expression: "\d{1,2}/\d{1,2}/\d{1,4}". Make sure you use matches() instead of find() to test that the pattern matches the full text.

Here's how you'd use ParsePosition:However, the regex solution is probably preferable since you'll likely already be using regex to validate the other fields.
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
your intervention was most helpfull; thanks a lot!

i guess i'll go with new regex(the other solution seems cant solve my prob):




Tue Mar 29 00:00:00 BST 2005
Tue Mar 01 00:00:00 GMT 2005

as an apart i found out that BST means British Summer Time
David Harkness
Ranch Hand

Joined: Aug 07, 2003
Posts: 1646
Originally posted by miguel lisboa:
You need to move the setLenient(false) call to before the parse() line; that's why it didn't catch the bad date. Once you do that, they work the same. And in fact, now that I think about it, if you're going to use the regex each time you parse a date, the second solution is faster.

You can speed up the regex by compiling it once and reusing the pattern. Also, if this class is going to be used in a single-threaded app (probably not likely), you can make the DateFormat static to avoid creating it every call. But DateFormat is not thread-safe, so be careful.
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
David,
one last Q:
You can speed up the regex by compiling it once and reusing the pattern

Can you explain, please?

TIA

edit:
i think i sorted it out, here
[ April 04, 2005: Message edited by: miguel lisboa ]
James Carman
Ranch Hand

Joined: Feb 20, 2001
Posts: 580
See the Pattern.compile( String regex ) method. It returns a reusable Pattern object.


James Carman, President<br />Carman Consulting, Inc.
 
GeeCON Prague 2014
 
subject: Please review my code