Why am I not getting the right return message?

Question

Below I have a snippet of code that I won't work. I get input in my main method then pass that input into another method to check for validation. But it doesn't really check correctly. If I input 99 for month and day I expect it to give me the message Check Month.

Instead I get: THIS THIS

If I input 02 for month and 99 for day, I expect it to give me the message: Check day. Instead I get THIS THIS

If I input 02 for both, I expect it to run and continue running other methods. Instead I get THIS THIS.

public class Date {

private Calendar parsedDate;

public static void main(String[] args) 
{
    Date main = new Date();
        System.out.println("Enter a date (use the format -> (MM/DD/YYYY)");

    //declare Scanner
    Scanner in = new Scanner (System.in);

    System.out.println("Enter a month (MM): ");
    String month = in.nextLine();

    System.out.println("Enter a day (DD): ");
    String day = in.nextLine();

    System.out.println("Enter a year (YYYY): ");
    String year = in.nextLine();

    if (main.isValidDate(month, day, year) == true) 
    {
        main.newFormat(month, day, year);
        main.isLeapYear(year);
        main.dayNumber(month, day);
    }
    else if (main.isValidDate(month, day, year) == false)
    {
        System.out.println("Invalid Input");
    }
}//end of main

private boolean isValidDate(String month, String day, String year) 
{
    //check month       
    if(month == "01" || month == "03" || month == "04" ||
       month == "05" || month == "06" || month == "07" || month == "08" ||
       month == "09" || month == "10" || month == "11" || month == "12")
    { 
        //check day
        if(day == "01" || day == "02" || day == "03" || day == "04" ||
           day == "05" || day == "06" || day == "07" || day == "08" ||
           day == "09" || day == "10" || day == "11" || day == "12" ||
           day == "13" || day == "14" || day == "15" || day == "16" ||
           day == "17" || day == "18" || day == "19" || day == "20" ||
           day == "21" || day == "22" || day == "23" || day == "24" ||
           day == "25" || day == "26" || day == "27" || day == "28" ||
           day == "29" || day == "30" || day == "31")
        {
            return true;
        }
        else
        {
            System.out.println("Check Day");
            return false;
        }
    }//end of check month
    else if (month == "02")
    {
        if (day == "28" || day == "29")
        {
            return true;
        }
    }//end of month 2
    else
    {
        System.out.println("THIS");
        return false;
    }

    parsedDate = null;// if it's valid set the parsed Calendar object up.
    return true;
}//end of isValidDate

score 5 · Answer 1 · answered Apr 03 '13 at 05:55

5

You should use the equals method for comparing Strings. When using the == operator, you compare the Strings' addresses, and not their content.

answered Apr 03 '13 at 05:55

Amir Kost

2,148
1
16
30

o. derp. >.< Thanks, I completely forgot! – Dwelling Place Apr 03 '13 at 05:58

score 4 · Answer 2 · answered Apr 03 '13 at 05:54

4

This month == "01" is not the correct way to compare Strings in Java.

You are trying to compare memory locations of the two variables, which aren't likely to be equal.

Instead you should be using month.equals("01").

answered Apr 03 '13 at 05:54

MadProgrammer

343,457
22
230
366

Can I use the || inside of that? ie `month.equals("01" || "02" || 03)` ? – Dwelling Place Apr 03 '13 at 05:59
@DwellingPlace No. In fact JonSkeet gave you an exellent suggestion. If you can parse the values to an `int` value first, then you can compare ranges `if (month >= 1 && month <= 3){...}` – MadProgrammer Apr 03 '13 at 06:00

Jon Skeet · Accepted Answer · 2013-04-03T06:24:21.913

4

While the answers explaining the reason it's currently not working - using == rather than equals - I would suggest you don't just start using equals.

Instead, parse the user input into numbers first, and then validate those... it's much easier to perform numeric comparisons with numbers rather than strings. Something like:

private boolean isValidDate(int year, int month, int day) {
    // Adjust for whatever bounds you want
    if (year < 1900 || year > 2100) {
        System.out.println("Check year");
        return false;
    }

    if (month < 1 || month > 12) {
        System.out.println("Check month");
        return false;
    }

    Calendar calendar = Calendar.getInstance();
    calendar.clear();
    calendar.set(year, month - 1, 1);
    if (day < 1 || day > calendar.getActualMaximum(Calendar.DAY_OF_MONTH)) {
        System.out.println("Check day");
        return false;
    }
    calendar.set(Calendar.DAY_OF_MONTH, day);
    // Store calendar somewhere if you want...
    return true;
}

Additionally, I'd strongly recommend the use of Joda Time as a much nicer date/time API if you can.

EDIT: Your next problem seems to be this block:

if (main.isValidDate(month, day, year) == true) 
{
    main.newFormat(month, day, year);
    main.isLeapYear(year);
    main.dayNumber(month, day);
}
else if (main.isValidDate(month, day, year) == false)
{
    System.out.println("Invalid Input");
}

Now the code I've presented already checks everything and constructs a Calendar - it's not clear what the methods in the if block are meant to do. But you don't need to call isValidDate a second time. You just need:

// Assuming you're using my new method...
if (isValidDate(year, month, day)) {
    // Do whatever you need to do
} else {
    System.out.println("Invalid input");
}

edited Apr 03 '13 at 06:24

answered Apr 03 '13 at 05:57

Jon Skeet

1,421,763
867
9,128
9,194

1

As Jon wrote you should declare constants and compare in this way `if (month==JANUARY)` etc. It's more clearly. – pepuch Apr 03 '13 at 05:59
If I put in the wrong month, it prints out `"Check Month"` twice! Why is this? – Dwelling Place Apr 03 '13 at 06:17
@DwellingPlace: Well currently you're calling `isValidDate` twice. Don't do that. – Jon Skeet Apr 03 '13 at 06:18
Not sure what I should do to not do that... Any help? ;x – Dwelling Place Apr 03 '13 at 06:20
@PatrykRoszczyniała: Where did I write anything about declaring a constant? `Calendar` already defines those constants, but I wouldn't want to check against each of them anyway... – Jon Skeet Apr 03 '13 at 06:25
@JonSkeet thanks! I see it now! :) If I were to reprompt for input, how would I do that? – Dwelling Place Apr 03 '13 at 06:29
@Jon, I've added my comment just before you've pasted the code. – pepuch Apr 03 '13 at 06:51
@PatrykRoszczyniała: Sure, but I *never* suggested declaring constants, so it's not clear where your comment came from at all... – Jon Skeet Apr 03 '13 at 07:22
@DwellingPlace: Well, you'd need a loop... but I'm not going to rewrite every bit of your code for you. – Jon Skeet Apr 03 '13 at 07:23
@JonSkeet I figured it out. I used a `while` after each requested entry and made additional methods to validate them. :) – Dwelling Place Apr 03 '13 at 07:47

score 2 · Answer 4 · answered Apr 03 '13 at 06:04

There are two big mistakes

you are comparing string with equals equals which will compare memory not content so the line if(month == "01") should be if("01".equals(month))
The condition for checking valid date if (main.isValidDate(month, day, year) == true) should be like if (main.isValidDate(month, day, year))

score 1 · Answer 5 · answered Apr 03 '13 at 05:59

1

It would be MUCH more efficient and easier to read/write/test to convert your string to integer and then check that (e.g.use int monthVal = Integer.parseInt(month); thenyou can check if (a) it parsed properly,with a try/catch around it and (b) test the value with if (monthVal > 0 && monthVal< 13) { /* month is good */}

answered Apr 03 '13 at 05:59

Steve Atkinson

1,219
2
12
30

I see! Can I still return the same way? – Dwelling Place Apr 03 '13 at 06:01
1

yes of course you can :) – Steve Atkinson Apr 03 '13 at 06:02
1

Good luck. pls come back and accept one of the answers when you have it working :) – Steve Atkinson Apr 03 '13 at 06:08

Why am I not getting the right return message?

5 Answers5