Question

I have a large group of nested IF statements and I was wondering If anyone had any suggestions on how to optimize for speed, size, and readability.

Below is a sample of ONE of the if statements and its' nested statements. There will be approximately 25-30 of these in the document.

if( $row["inlet_moisture"] > $row["inlet_moisture_high_warning"] ) {
    if( $row["inlet_moisture"] > $row["inlet_moisture_high_critical"] ) {
        if( $row["inlet_high_critical"] == 0 ) {
            if( $row["email_notification"] == 1 ) {

            }
            if( $row["mobile_notification"] == 1 ) {

            }
        }
    } else {
        if( $row["inlet_high_warning"] == 0 ) {
            if( $row["email_notification"] == 1 ) {

            }
            if( $row["mobile_notification"] == 1 ) {

            }
        }
    }
} else if( $row["inlet_moisture"] < $row["inlet_moisture_low_warning"] ) {
    if( $row["inlet_moisture"] < $row["inlet_moisture_low_critical"] ) {
        if( $row["inlet_low_critical"] == 0 ) {
            if( $row["email_notification"] == 1 ) {

            }
            if( $row["mobile_notification"] == 1 ) {

            }
        }
    } else {
        if( $row["inlet_low_warning"] == 0 ) {
            if( $row["email_notification"] == 1 ) {

            }
            if( $row["mobile_notification"] == 1 ) {

            }
        }
    }
}

The idea is; I have a reading (temp/speed/moisture) and I need to check if it hits any of the limits (high warning / high critical / low warning / low critical), if it does I first need to check if I have already sent an alarm for that. If no alarm has been sent I then need to check if the user has requested alarm notification (mobile/email/both)

Currently this works. I Just don't like how heavy it is? Can I improve on this?

Thanks.

Was it helpful?

Solution

this seems to me much more clear, even though you could combine the nested ifs I'd rather prefer this one

if( $row["inlet_moisture"] > $row["inlet_moisture_high_critical"] ) {
  if( $row["inlet_high_critical"] == 0 ) {
   $message = 'the pertinent message';
  }
}
else if( $row["inlet_moisture"] > $row["inlet_moisture_high_warning"] ) {
  if( $row["inlet_high_warning"] == 0 ) {
   $message = 'the pertinent message';
  }
}
else if( $row["inlet_moisture"] < $row["inlet_moisture_low_critical"] ) {
  if( $row["inlet_low_critical"] == 0 ) {
   $message = 'the pertinent message';
  }
}
else if( $row["inlet_moisture"] < $row["inlet_moisture_low_warning"] ) {
  if( $row["inlet_low_warning"] == 0 ) {
   $message = 'the pertinent message';
  }
}


if( $row["email_notification"] == 1 ) {
  sendMessage($message, $email);
}
if( $row["mobile_notification"] == 1 ) {
  sendMessage($message, $mobile);    
}

OTHER TIPS

Premature optimisation is the root of all evil - with what we are dealing with here, no matter what you do it won't have much/any noticable impact on performance.

Having said that, a large amount of if statements can often be replaced with one or more switch structures, although whether this improves performance or readability is debatable. You may also be able to create some functions for repeated bits of code, although this may actually negatively impact performance.

From your comment above... creating variables with nicer names will have pretty much zero impact on performance. If will slightly increase your memory usage, but the processing time impact will be next to zero. And, if you are evaluating the values as booleans, you don't need to convert them to booleans explicitly, because 1 still evaluates to TRUE, and 0 to FALSE. However, if you do want to do it

$email_notification = $row["email_notification"] == 1 ? true : false;

...is unnecessarily long winded, you can do either this:

$email_notification = $row["email_notification"] == 1;

...or...

$email_notification = (bool) $row["email_notification"];

...and it will have the same effect.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top