Style: Breaking up long if statements

Andrew Bennett <andrew.bennett@unsw.edu.au>, COMP1511 18s1 Course Forum, 19T1 Course Forum

Breaking up long if statements

Student:

Which style is better?

I'm wondering which one of this is preferred:

1: a long statement with && ||

e.g. (jhsjdh && whjhjr && dfhhdhfdhdkkj&& fjhufhh) ||(aghjGHASGFS && dshfhjasf && dkjfkkdg) ||(hfgdsfhj && fhasjfhu) || (hhfkjshf)

2: break the above statements down by using if and else if ( which takes more lines)

Andrew:

Using more lines is (usually) never a bad thing.

There is also a requirement in the style guide that lines not be more than 80 characters wide. If your line is hundreds or even thousands of characters wide, you will have serious style marks deducted.

That said, I wouldn't consider something like this to be very good style, whether or not you split it across multiple lines:

if ((.......... && .......... && .........) ||
    (........&& ..........) || (........ || ........) ||
    (.......... && ....... && ........)) {
    // do something
}

Having a long chain of if statements like that isn't very readable.

A better way to do it might be to put the logic of those if statements into a function, and instead make your code be like

if (someFunction(....)) {
    // do something
}

And then inside someFunction, making the code be more readable in there too, e.g. this would not be very good style:

int someFunction(...) {

    int something = FALSE;

    if ((.......... && .......... && .........) || 
        (........&& ..........) || (........ || ........) || 
        (.......... && ....... && ........)) {

        something = TRUE;
    }

    return something;
}

It would make more sense to break it up further to be more readable, e.g.

int someFunction(...) {
    int something = FALSE;
    if (.......) {
        something = TRUE;
    } else if (.....) {
        something = TRUE;
    } else if (......) {
        // etc etc etc
    }
    return something;
}

You wouldn't necessarily have to break every single condition into a separate if statement, but you should be able to do it sensibly.

Thinking back to the leapYear example, you could write something like this...

if (year % 400 == 0) {
    isLeapYear = TRUE;
} else if (year % 100 == 0) {
    isLeapYear = FALSE;
} else if (year % 4 == 0) {
    isLeapYear = TRUE;
} else {
    isLeapYear = FALSE;
}

Or something like this...

if (((year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0)) {
 isLeapYear = TRUE;
} else {
 isLeapYear = FALSE;
}

They both achieve the same thing, but reading the first one is (in my opinion) significantly clearer than the second one.

So if we are unable to use the multiple if statement version of this and our statement is made up of 5 &&'s or something like that what is the best thing to do style-wise, aside from using a function since the code would still be messy within the function anyway?

You can make the if statement chain within the function clearer (see the leap year example above).

Taking your example:

// Without limit
if (condition1 == num1 && condtion2 < num2 && condtion4 != num3 && condition5 == num5) {
    variable1 = 1;
} else if (condition6 == num6 && condition7 < num7 && condition8 != num8 && condition9 % 2 == 0) {
    variable1 = 2;
} else if (condition10 == num10 && condition11 < num11 && condition12 != num12) {
    variable1 = 3;
else {
    variable1 = 0;
}

This could become:

if (the_sky_is_blue(...)) {
    variable1 = 1;
} else if (it_is_raining(...)) {
    variable1 = 2;
} else if (the_sun_is_on_fire(...)) {
    variable1 = 3;
else {
    variable1 = 0;
}

And then the functions could become something like:

// this is one example of how you could structure the if statement within a function
int some_thing_to_check(...) {
    return (condition1 == num1
        && condtion2 < num2
        && condtion4 != num3
        && condition5 == num5);
}

// could also structure it like this
int some_other_thing_happened(...) {
    return condition6 == num6 &&
           condition7 < num7 && 
           condition8 != num8 && 
           condition9 % 2 == 0;
}

// or something like this (or any other number of ways)
int the_sun_is_on_fire(...) {
    int is_on_fire = FALSE;
    if (condition10 == num10
        && condition11 < num11
        && condition12 != num12) {

        is_on_fire = TRUE;
    }

    return is_on_fire;
}