Not always getting expected outcome when Comparing Calendar objects

[Jd Wells]

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.

private static String determineSign(Date birthdate, String[][] zodiac) throws ParseException {
		// TODO Auto-generated method stub
			Date date = birthdate;
		
		
		for (int row = 0; row < zodiac.length; row++)
		{
			DateFormat date3 = new SimpleDateFormat("MM/dd");
			Calendar compDate3 = Calendar.getInstance();
			compDate3.setTime(date3.parse(zodiac[row][2]));
			Calendar convDate3 = compDate3;
			
			
			compDate3.clear(Calendar.HOUR);
			compDate3.clear(Calendar.HOUR_OF_DAY);
			compDate3.clear(Calendar.MINUTE);
			compDate3.clear(Calendar.SECOND);
			compDate3.clear(Calendar.MILLISECOND);
			compDate3.clear(Calendar.YEAR);
			
			
			DateFormat date2 = new SimpleDateFormat("MM/dd/");
			Calendar compDate2 = Calendar.getInstance();
			compDate2.setTime(date2.parse(zodiac[row][1]));
			Calendar convDate2 = compDate2;
		
			compDate2.clear(Calendar.HOUR);
			compDate2.clear(Calendar.HOUR_OF_DAY);
			compDate2.clear(Calendar.MINUTE);
			compDate2.clear(Calendar.SECOND);
			compDate2.clear(Calendar.MILLISECOND);
			compDate2.clear(Calendar.YEAR);
			
			
			Calendar compDate1 = Calendar.getInstance();
			DateFormat format = new SimpleDateFormat("MM/dd/yyyy");
			format.format(date);
			compDate1 = format.getCalendar();
			
			compDate1.clear(Calendar.HOUR);
			compDate1.clear(Calendar.HOUR_OF_DAY);
			compDate1.clear(Calendar.MINUTE);
			compDate1.clear(Calendar.SECOND);
			compDate1.clear(Calendar.MILLISECOND);
			compDate1.clear(Calendar.YEAR);
			

			
			if (compDate1.equals(compDate2) ||
                        compDate1.after(compDate2) && compDate1.equals(convDate3) ||
                        compDate1.before(compDate3) )
			{
				
				Date oldDate2 = convDate2.getTime();
				Date oldDate3 = convDate3.getTime();
                
				Zodiac yourSign = new Zodiac(zodiac[row][0], oldDate2, oldDate3);
				
				String returnString = "Your zodiac sign is "
                               + yourSign.getName() + " " + yourSign.getStartDate() + " "
                               + yourSign.getEndDate();
				return returnString;
			}
			
			else 
			{
				System.out.println("Number is out of bounds. Try again.");
				System.exit(0);
			}
		}
		return null;
	}

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

public class ZodiacTable {
	static final int ROWS = 12;
	static final int COLUMNS = 3;
	//private ArrayList<String> zodiacNames;
	
	private String[][] zodiacNames;
	
	
    //private ZodiacTable zodiac = (ZodiacTable)zodiacNames;
	
	public ZodiacTable() {
		zodiacNames = new String[ROWS][COLUMNS];
		
		zodiacNames[0][0] = "Aries";
		zodiacNames[0][1] = "03/21/2014";
		zodiacNames[0][2] = "04/20/2014";
		
		zodiacNames[1][0] = "Taurus";
		zodiacNames[1][1] = "04/21/2014";
		zodiacNames[1][2] = "05/20/2014";
		
		zodiacNames[2][0] = "Gemini";
		zodiacNames[2][1] = "05/21/2014";
		zodiacNames[2][2] = "06/21/2014";
		
		zodiacNames[3][0] = "Cancer";
		zodiacNames[3][1] = "06/22/2014";
		zodiacNames[3][2] = "07/22/2014";
		
		zodiacNames[4][0] = "Leo";
		zodiacNames[4][1] = "07/23/2014";
		zodiacNames[4][2] = "08/23/2014";
		
		zodiacNames[5][0] = "Virgo";
		zodiacNames[5][1] = "08/24/2014";
		zodiacNames[5][2] = "09/23/2014";
		
		zodiacNames[6][0] = "Libra";
		zodiacNames[6][1] = "09/24/2014";
		zodiacNames[6][2] = "10/23/2014";
				
		zodiacNames[7][0] = "Scorpio";
		zodiacNames[7][1] = "10/24/2014";
		zodiacNames[7][2] = "11/22/2014";
		
		zodiacNames[8][0] = "Sagittarius";
		zodiacNames[8][1] = "11/23/2014";
		zodiacNames[8][2] = "12/21/2014";
		
		zodiacNames[9][0] = "Capricorn";
		zodiacNames[9][1] = "12/22/2014";
		zodiacNames[9][2] = "01/20/2014";
		
		zodiacNames[10][0] = "Aquarius";
		zodiacNames[10][1] = "01/21/2014";
		zodiacNames[10][2] = "02/19/2014";
		
		zodiacNames[11][0] = "Pisces";
		zodiacNames[11][1] = "02/20/2014";
		zodiacNames[11][2] = "03/20/2014";	
	}
	

	/**
	 * @return the zodiacNames
	 */
	public String[][] getZodiacNames() {
		return zodiacNames;
	}

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

thanks

[Jan Hoppmann]

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]

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]

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]

Winston Gutkowski wrote: . . . I would suggest using one of the new Date/Time classes . . .

As discussed in JD Wells' other thread?

[Jd Wells]

Thanks Winston... That was very helpful!

jd

[Winston Gutkowski]

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]

will do.

[Winston Gutkowski]

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]

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

package zodiac;

import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.*;
/** Driver program for Zodiac
 * 
 * @author joew
 *
 */
public class ZodiacDriver {
	
       
    
	public static void main(String[] args) throws ParseException {
		// TODO Auto-generated method stub
            Date birthdate = getBirthdate();
			ZodiacTable zodiac = createZodiacTable();
			
			try
			{
			System.out.println(determineSign(birthdate,zodiac));
			}
			catch (NullPointerException npRef)
			{
				System.out.println("Line 18:" + npRef.toString());
			}
	}
	private static String determineSign(Date birthdate, ZodiacTable zodiac) throws ParseException {
		// TODO Auto-generated method stub
			System.out.println(zodiac);
			String returnString = null;
			Date date = birthdate;
			//System.out.println(zodiac.g);
			Calendar compDate = Calendar.getInstance();
			DateFormat format = new SimpleDateFormat("MM/dd/");
			format.format(date);
			compDate = format.getCalendar();
			
			compDate.clear(Calendar.HOUR);
			compDate.clear(Calendar.HOUR_OF_DAY);
			compDate.clear(Calendar.MINUTE);
			compDate.clear(Calendar.SECOND);
			compDate.clear(Calendar.MILLISECOND);
			compDate.clear(Calendar.YEAR);
			
				
		//test:
		for (ZodiacSign z : zodiac.getZodiac())
              
			
		{
			Calendar startDate = Calendar.getInstance();
			format.format(z.getStartDate());
			startDate = format.getCalendar();
			
			startDate.clear(Calendar.HOUR);
			startDate.clear(Calendar.HOUR_OF_DAY);
			startDate.clear(Calendar.MINUTE);
			startDate.clear(Calendar.SECOND);
			startDate.clear(Calendar.MILLISECOND);
			startDate.clear(Calendar.YEAR);
			
			Calendar endDate = Calendar.getInstance();
			format.format(z.getEndDate());
			endDate = format.getCalendar();
			
			endDate.clear(Calendar.HOUR);
			endDate.clear(Calendar.HOUR_OF_DAY);
			endDate.clear(Calendar.MINUTE);
			endDate.clear(Calendar.SECOND);
			endDate.clear(Calendar.MILLISECOND);
			endDate.clear(Calendar.YEAR);
				
			
			
			//if (compDate.before(startDate) && compDate.after(endDate))
							
				//{
					//continue test;
				//}
			
						
			 if (compDate.equals(startDate))
									
			{
				
			 
			System.out.println(startDate);
			System.out.println(endDate);
			System.out.println(compDate);	
				returnString = "Your zodiac sign is " + z.getName() + " " + z.getStartDate() + " "
				+ z.getEndDate();
				 
				 
				
               }			
		}
			
		return returnString;
		
	}

	public static ZodiacTable createZodiacTable() throws ParseException {
		 
		 SimpleDateFormat f = new SimpleDateFormat("MM/dd/yyyy"); 
		
		 ZodiacSign aries = new ZodiacSign("Aries",f.parse("03/21/2015"),f.parse("04/20/2015"));
		 ZodiacSign taurus = new ZodiacSign("Taurus", f.parse("04/21/2015"), f.parse("05/20/2015"));
		 ZodiacSign gemini = new ZodiacSign("Gemini", f.parse("05/21/2015"),f.parse("06/21/2015"));
		 ZodiacSign cancer = new ZodiacSign("Cancer",f.parse("06/22/2015"),f.parse("07/22/2015"));
		 ZodiacSign leo = new ZodiacSign("Leo",f.parse("07/23/2015"),f.parse("08/23/2015"));
		 ZodiacSign virgo = new ZodiacSign("Virgo",f.parse("08/24/2014"),f.parse("09/23/2014"));
		 ZodiacSign libra = new ZodiacSign("Libra", f.parse("09/24/2014"), f.parse("10/23/2014"));
		 ZodiacSign scorpio = new ZodiacSign("Scorpio", f.parse("10/24/2014"), f.parse("11/22/2014"));
         ZodiacSign saggit = new ZodiacSign("Sagittarius", f.parse("11/23/2014"), f.parse("12/21/2104"));
         ZodiacSign capri = new ZodiacSign("Capricorn", f.parse("12/22/2014"), f.parse("01/20/2015"));
         ZodiacSign aquar = new ZodiacSign("Aquarius", f.parse("01/21/2015"), f.parse("02/19/2015"));
         ZodiacSign pisces = new ZodiacSign("Pisces", f.parse("02/20/2015"), f.parse("03/20/2015"));
         
         ZodiacTable table = new ZodiacTable();
                  
         
          table.addZodiac(aries);
          table.addZodiac(taurus);
		  table.addZodiac(gemini);
		  table.addZodiac(cancer);
		  table.addZodiac(leo);
		  table.addZodiac(virgo);
		  table.addZodiac(libra);
		  table.addZodiac(scorpio);
		  table.addZodiac(saggit);
		  table.addZodiac(capri);
		  table.addZodiac(aquar);
		  table.addZodiac(pisces);

			 System.out.println(table);
		 return table;
		 
		}
	
	public static  Date getBirthdate() throws ParseException {
		Date bdate = null;
		Scanner keyboard = new Scanner(System.in);					
		System.out.println("Please enter your birthdate.(Format 03/28/1968)");
		String birthDate = keyboard.next();
		
		SimpleDateFormat f = new SimpleDateFormat("MM/dd/yyyy");
		f.setLenient(false);
		
		try 
		{
			bdate = f.parse(birthDate);
			keyboard.close();
			//return date;
		}
		
		catch (ParseException peRef)
		{
			System.out.println("Line 99: Exception"
					+ peRef.toString() + " " + "Invalid Date. Goodbye");
			System.exit(0);
		}
		
		System.out.println(bdate);
		return bdate;
		
	}
		

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]

Crossposted on the Java Forum:
http://www.java-forums.org/new-java/92897-date-compasrisons-not-working.html

[Campbell Ritchie]

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]

Dave Tolls wrote:Crossposted on the Java Forum:
http://www.java-forums.org/new-java/92897-date-compasrisons-not-working.html

Thank you for noticing.

[Dave Tolls]

For some unknown reason I had a thought about this on the way in this morning.

DateFormat format = new SimpleDateFormat("MM/dd/");  
format.format(date);  
compDate = format.getCalendar();

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

format.format(z.getStartDate());  
startDate = format.getCalendar()

^
|
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]

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]

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:

DateFormat mmdd = new SimpleDateFormat("MM/dd");  
Date d = mmdd.parse("02/29");

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]

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]

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):

if (compDate.equals(startDate))

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]

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