• Post Reply Bookmark Topic Watch Topic
  • New Topic

Not always getting expected outcome when Comparing Calendar objects  RSS feed

 
Jd Wells
Greenhorn
Posts: 15
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,
I am writing a short program to find the zodiac of a given birth date. I am checking to see if a calendar object falls between to other objects to see if the date falls within a certain zodiac sign.
To accomplish this, I have the user enter a date, then that date is compared to dates taken from an array. The problem is sometimes the comparison doesnt seem to work work, giving the wrong zodiac. For example if I enter 03/28/1968, it finds Aries, as expected. However if I enter 03/20/1968, it returns Aries also, when it should return Pisces. Here is the sample code.




This is the class that creates the table used for the zodiac dates -



I cant seem to see why it returns the wrong zodiac.. maybe I just need a second set of eyes.. lol

thanks
 
Jan Hoppmann
Ranch Hand
Posts: 147
Android Eclipse IDE Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have to say that I have difficulty following your code. For starters, compDate1 through 3 are not good names for variables. Why not something like birthdate, zodiacStart, zodiacEnd or something like this? That would make the logic in line 49 much clearer. And why do you parse the year of date in line 36, only to throw it away in line 45?
 
Campbell Ritchie
Marshal
Posts: 56570
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The long lines do not help; I have broken them. you should use spaces for indenting and align the broken lines so the operators or the quotes align vertically. It would look a lot better like that.
Your array of arrays is dreadful design and should be changed. You should have Zodiac or Sign objects. Each can encapsulate start and end dates. Another bad sign is using Strings instead of dates. As you will see from this FAQ Strings are designed for writing and a date is a date not writing. Also returning a reference to that array is dangerous. There is nothing to stop somebody writing
myZodiacTable.getZodiacNames()[3] = new String{"Tom", "Dick", Harry"};
Why are the dates all this year?
There is a far better way to write Zodiac objects: with an enum.
I see you have a method which can return null. That is also dangerous. What happens when somebody tries to use the return value of that method? You can suffer an Exception.

Those if (something || something && something || something)… statements are very error‑prone. Write it out very carefully in the format p ∨ q ∧ r ∨ s and use underlining or brackets or something to mark which operator has the highest precedence. I mean, if you think the left ∨ has the highest precedence, write it out like this:- p ∨ q ∧ r ∨ s.
Remember you may have to consider both precedence and associativity of operators. ∨ = or ∧ = and. Both associate to the left. Precedences can be found out here. Look for conditional or and conditional and. When I said or I meant “inclusive or” throughout.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jd Wells wrote:I cant seem to see why it returns the wrong zodiac.. maybe I just need a second set of eyes.. lol

Well, first up: I agree with Jan - your code is quite confusing.

Second: You're going to a LOT of trouble to return a standardized "calendar day", and I'm not at all sure it will work - although I can't actually pinpoint your specific problem just at the moment.

However, it's worth noting that the docs of the clear(int) method state: "date and time calculations will treat the field as if it had never been set"; and it actually goes on to point out that in the case of HOUR, it does NOT mean the same as setting it to 0. In fact it may not mean that for ANY of the fields since, if they're "not set", they may well use the "current time" as their value. The docs aren't clear on this though.

Simply put: Calendar is a horrible interface, and why it wasn't trashed long ago is a mystery to many of us.

If you're on version 8, then I would suggest using one of the new Date/Time classes (not exactly sure which one, but I'm pretty certain there is one); but if you're not, then you could roll your own CalendarDate class by subclassing GregorianCalendar, and overriding its compareTo() method with one that ONLY compares MONTH and DAY_OF_MONTH fields.

It's something a lot of people forget: many of the Java foundation classes aren't final.

HIH

Winston

[Edit] Alternatively: Create a utility class (Calendars?) that includes a compareByDate(Calendar c1, Calendar c2) method. That might actually be better, since an instance method won't be symmetrical.
 
Campbell Ritchie
Marshal
Posts: 56570
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote: . . . I would suggest using one of the new Date/Time classes . . .
As discussed in JD Wells' other thread?
 
Jd Wells
Greenhorn
Posts: 15
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Winston... That was very helpful!

jd
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jd Wells wrote:Thanks Winston... That was very helpful!

You're most welcome. Dates and times aren't easy - oddly enough, because computers are so simple - it's US that have the problems.

For more info, you might want to look at the ABriefHistoryOfJavaTime page. It doesn't contain any specific stuff about "calendar dates", but hopefully you'll get some idea that most of our problems stem from what we want to DO with "time", not from what is IS.

HIH

Winston
 
Jd Wells
Greenhorn
Posts: 15
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
will do.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:Alternatively: Create a utility class (Calendars?) that includes a compareByDate(Calendar c1, Calendar c2) method. That might actually be better, since an instance method won't be symmetrical.

Actually, thinking about it, a Comparator is probably even better. I'll leave you to work out how to do it; come back if you run into problems.

Winston
 
Jd Wells
Greenhorn
Posts: 15
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the help Winston. I actually re-wrote most of the program. I got it working the way it was, but it really needed some work and I wasn't meeting other requirements for the project. Now that I have re-done it,
I cant seem to get my if statements which are comparing the dates to work. When I enter a date, It just prints out the last item in my arrayList, not matter what date I enter. I think I just need another set of eyes, cause I seem to be going in circles with it. Here is my driver program




All Im trying to get it to do first of all is to print out a sign if the date entered matches a date in the table. I get pisces every time.
 
Dave Tolls
Ranch Foreman
Posts: 3065
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Crossposted on the Java Forum:
http://www.java-forums.org/new-java/92897-date-compasrisons-not-working.html
 
Campbell Ritchie
Marshal
Posts: 56570
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And how have you investigated the working of the new program?
There must be a better way to use dates than parsing Strings. Look in the Java Tutorials for the newer date classes.
And what did you tell the people on the other forum? What did they tell you?
 
Campbell Ritchie
Marshal
Posts: 56570
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you for noticing.
 
Dave Tolls
Ranch Foreman
Posts: 3065
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
For some unknown reason I had a thought about this on the way in this morning.

^
|
Here you get the Calendar from the SDF, which contains the date to compare.


^
|
For each Zodiac you format the Date (and do nothing with the returned String) and then get the Calendar again, from the same SDF.

SimpleDateFormat is not thread safe (yes, this has nothing to do with thread safety, but bear with me). There's an ancient bug raised against it, querying why on earth it can't be. And the reason it's not thread safe is because it has an internal Calendar that it uses to help with the formatting. This is the Calendar returned by getCalendar(). And consequently compDate, startDate, endDate and the Calendar in 'format' are all referencing the same Calendar object.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dave Tolls wrote:For each Zodiac you format the Date (and do nothing with the returned String) and then get the Calendar again, from the same SDF.

Right, but I think (still not absolutely sure) that Jd's problem stems from the fact that he was comparing those Calendars with compareTo(), which takes ALL fields into account - and the simple fact is, I have no idea what happens when you create a Date from a format like "MM/dd".

I suspect that it will plug in values for the other fields based on System.currentTimeMillis(), but I'm just not sure. But if I'm right, and you then create a Calendar from that, then the time fields in those Dates might be different.
Now originally, he was clear()ing all those fields once he'd set up the Calendar, which makes sense; but again, I'm not quite sure exactly what the ramifications of that are - certainly the method docs indicate that clearing HOUR is not necessarily the same as setting it to 0.

The fact is that the only things in play here are month and day of month, so I think if I wasn't going to use one of the new v8 classes, I'd either write my own MonthDate class, or use a Comparator.

Winston
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:and the simple fact is, I have no idea what happens when you create a Date from a format like "MM/dd".

I must actually try this. What happens if you write:
Do you get an Exception? Do you get d = 2014/03/01...?

I'll let you know if/when I get around to it.

Winston
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dave Tolls wrote:And consequently compDate, startDate, endDate and the Calendar in 'format' are all referencing the same Calendar object.

I think I see what you're getting at now - sorry, I was being a bit thick there.

But doesn't that suggest that you're then changing that Calendar? So in order to do a comparison, you'd then have to get another Date from its current state, wouldn't you?

Fun stuff. At the end of the day, it's only so involved because Calendar is so arcane though.

Winston
 
Dave Tolls
Ranch Foreman
Posts: 3065
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No problem.
The thing is, in the second code above (which is the only code posted at the Java Forums) the comparison is done (in the while loop):

Not only are compDate and startDate Calendars, they're the same Calendar...so that's always true.
Since there is no 'break;' from that loop, the end result is that the last Zodiac is always chosen.

It's not (IMO) that Calendar is arcane (though that doesn't help) it's the (ab)use of SDF to do this job.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dave Tolls wrote:It's not (IMO) that Calendar is arcane (though that doesn't help) it's the (ab)use of SDF to do this job.

That actually occurred to me too.

It seems to me that the whole thing could actually be done just with Dates if you don't mind using deprecated methods. After all, like I say, the only things in play are month and day, so just use String.split("/") and plough the results into a Date using something like 'new Date(2000, month-1, day)' (with a bit of validation first of course). Then you can just compare them.

Winston
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!