Using substrings properly in an if else statement java

Question

I am wondering if my logic and syntax makes sense with this if else statement? Do all of my if statements make sense when looking at syntax? I am trying to take the first two numbers of rentalDate to determine the month and see which season charge they will get. Thanks!

public double determinePrice () {

    double price;

    if (rentalDate.substring(0,2) == ("01") || rentalDate.substring(0,2) == ("02"))
    {
        price = WINTER_CHARGE;
    }
    else if (rentalDate.substring(0,2) == ("12"))
    {
        price = WINTER_CHARGE;
    }
    else if (rentalDate.substring(0,2) == ("03") || rentalDate.substring(0,2) == ("04") || rentalDate.substring(0,2) == ("05"))
    {
        price = SPRING_CHARGE;
    }
    else if (rentalDate.substring(0,2) == ("06") || rentalDate.substring(0,2) == ("07") || rentalDate.substring(0,2) == ("08"))
    {
        price = SUMMER_CHARGE;
    }
    else
    {
        price = FALL_CHARGE;
    }

    return price;   
}

score 0 · Accepted Answer · edited May 23 '17 at 11:52

No, they don't make sense. You're comparing the strings using reference equality, which is a common mistake when new to Java.

You should change all of your statements to use the .equals() method. This will compare the values of the Strings, as opposed to the values of the references, which you can also think of as "pointers" if you're coming from a C/C++ background.

So essentially, this:

rentalDate.substring(0,2) == ("01")

becomes...

"01".equals(rentalDate.substring(0,2))

You can read more here: https://stackoverflow.com/a/513839/88111

Reut Sharabani · Answer 2 · 2015-10-07T00:53:30.687

0

I would use a switch after parsing the month:

// can throw NumberformatException
int rentalMonthInt = Integer.parseInt(rentalDate.substring(0,2));

switch(rentalMonthInt){
    case 1:
    case 2:
    case 12:
        return WINTER_CHARGE;
    // add more groups...
    default:
        System.out.err("month out of range..." + rentalMonthInt);
        // handle bad month
}

Note that cases for 1, 2 and 12 are executing the same code since there are no instructions or breaks in any of them before the last. You can group months this way.

edited Oct 07 '15 at 00:53

answered Oct 07 '15 at 00:50

Reut Sharabani

30,449
6
70
88

1

Good answer - my only critique would be to group the months together, so that `case 1:`, `case 2:`, and `case 12:` all result in the same codeblock, `// december, january, or february` – Craig Otis Oct 07 '15 at 00:53
@CraigOtis I'm parsing his ranges, I'm not good with seasons. I'll just post an example. – Reut Sharabani Oct 07 '15 at 00:54

Using substrings properly in an if else statement java

2 Answers2