HDSB-eLearning-ICS4UO-202021 / Class-Examples

MIT License
0 stars 4 forks source link

Could the code within this if/else statement be written more efficiently? #1

Open hdsbbrooks opened 3 years ago

hdsbbrooks commented 3 years ago

This section of code within the if/else statement could be written in different ways. Rewrite this code in another way to achieve the same functionality. Do you think your version is more/less/equivalent efficiency to the original code? Explain why you think this.

https://github.com/HDSB-eLearning-ICS4UO-202021/Class-Examples/blob/6278f35bf97992aff53c3bbca6cb7f46c062f45d/Intro%20to%20OO/Game%20of%20Life/Cell.java#L86-L106

Dani3lx commented 3 years ago

Here is the new code

   if (living) {

        if ( numberNeighboursLiving == 2 || numberNeighboursLiving == 3 ) {
            return true;
        }
        else {
            return false;
        }

    }
    else {
         if (numberNeighboursLiving == 3) {
             return true;
         }
    }

This code is more efficient as it eliminates the need for else-if statement.

nachoroo commented 3 years ago

If you really wanted to save lines, you could simplify the if-else statement and rewrite the code with the ternary operator:

if (living) {
    return ( numberNeighboursLiving >= 2 && numberNeighboursLiving <= 3 ) ? true : false;  
} else {
    if (numberNeighboursLiving == 3) {
        return true;
    }
}

In effect, this code would be slightly more efficient, as it combines both survival criteria into one expression and reduces the nested if-then-else statement. The code is also a bit more compact, however, it is a little harder to read. If you were looking to write as few lines as possible, you could reduce it even further to this:

return living ? (numberNeighboursLiving >= 2 && numberNeighboursLiving <= 3) ? true : false : (numberNeighboursLiving == 3) ? true : false;

but then this becomes very impractical, messy, and hard to understand.

bepapab commented 3 years ago
if (living) {
           switch (numberNeighboursLiving) {
                case 2: return true;
                case 3: return true;
                default: return false; 
            }
        }
        else { //Cell is dead
             if (numberNeighboursLiving == 3) {
                 return true;
             }
        }

This code is easier to read and more effecient because it only does 2 comparisons.

ParmesanCheese345 commented 3 years ago
 if (living) {
        if (numberNeighboursLiving == 2 || numberNeighboursLiving == 3 ) 
            return true;
        else 
            return false;
    }
    else {
         if (numberNeighboursLiving == 3) 
             return true;
    }

My code is like previously posted solutions. All that is required is to merge the if and else-if statements into one if statement. This code can be written more quickly and is slightly more efficient. However, the increased efficiency will only be noticeable when repeating the code hundreds of thousands of times.

GalaxyBoy1234 commented 3 years ago

Effiency.txt

The code is easier to read and is more understandable

jules-77 commented 3 years ago

I combined the if and else if lines so that the result of them being true, returns true. This way it seems like there are less events that could occur. The changes would make the program more efficient because there is not an else-if statement. With that being said, that is such a small change it would be essentially unnoticeable. Correction.txt

1ligra commented 3 years ago
 if (living) { 
    return (numberNeighoursLiving == 2 || numberNeighoursLiving == 3) ? true : false;
} else {
    if (numberNeighoursLiving == 3) {
        return true;
    }
}

I basically just condensed the first part using operators.

Cheeseburger1776 commented 3 years ago

I agree with all the above answers. The code becomes more concise and efficient. Here is another option;

It keeps the code easy to read, but not less efficient. Anyways, its an option for the programmer.

Correction.txt

VernierLabQuest commented 3 years ago

I had the same answer basic answer as several people above, in simply combining the inner if statements.

        if (living) {
            if ( numberNeighboursLiving < 2||numberNeighboursLiving > 3 ) {
                return false;
            }
            //* Any live cell with two or three live neighbours lives on to the next generation.
            else if ( numberNeighboursLiving <= 3 ) {
                return true;
            }
             //* Any live cell with more than three live neighbours dies, as if by overpopulation.
        }
        else { //Cell is dead
             //* Any dead cell with exactly three live neighbours becomes a live cell, as if by reproduction.
             if (numberNeighboursLiving == 3) {
                 return true;
             }
        }

Alternatively, a switch statement could also work inside the if statement.

       if (living) {
            //* Any live cell with fewer than two live neighbours dies, as if caused by underpopulation.
            switch(numberNeighboursLiving){
                case 2:
                case 3:
                return true;
                default:
                return false;
            }
        }
        else { //Cell is dead
             //* Any dead cell with exactly three live neighbours becomes a live cell, as if by reproduction.
             if (numberNeighboursLiving == 3) {
                 return true;
             }
        }

Either way, the code runs the same with slight speed improvements.