What is the best method to write long if - else conditions?

[thara]

Can I know from the professionals here,  what is the best method to write this kind of logic? And also I would like to know is there other way to write this in a best way.

 

Method 01:

if ($emission <= 100 ) {
    $cssClass = "emission_a";
} elseif ($emission <= 120 ) {
    $cssClass = "emission_b";
} elseif ($emission <= 150 ) {
    $cssClass = "emission_c";
} elseif ($emission <= 165 ) {
    $cssClass = "emission_d";
} elseif ($emission <= 185 ) {
    $cssClass = "emission_e";
} else {
    $cssClass = "emission_g";
}

Method 02:

$mapping = array(
    100 => 'emission_a',
    120 => 'emission_b',
    150 => 'emission_c',
    165 => 'emission_d',
    185 => 'emission_e',
    225 => 'emission_f'
);

$cssClass = 'emission_g'; // default class if $emission is > 225

foreach ($mapping as $limit => $class) {
    if ($emission <= $limit) {
            $cssClass = $class;
            break;
    }
}

Thank you.

Edited October 21, 2015 by thara

[Barand]

As you are testing for a range of values (with <= ) then either is fine, although my preference would be "ifelse" in this instance.

 

If you were using "=" then I would definitely go for an array, as then you could just use

$cssClass = $mapping[$emission];

 

That's my 0.02 worth.

[thara]

@Barand, Can't we write this using "switch Statement"?

Edited October 21, 2015 by thara

[Ch0cu3r]

Yes you can however you need to write the switch statement differently. Like so

// matches which 'case' evaluates to true
switch(true)
{
    case($emission <= 100 ):
        $cssClass = "emission_a";
    	break;
    case($emission <= 120 ):
        $cssClass = "emission_b";
    	break;
    case($emission <= 150 ):
        $cssClass = "emission_c";
    	break;
    case($emission <= 165 ):
        $cssClass = "emission_d";
    	break;
    case($emission <= 185 ):
        $cssClass = "emission_e";
    	break;
    default:
        $cssClass = "emission_g";
    	break;
    }
}

[Jacques1]

You should avoid degenerate switch statements like that, though. They're extremely confusing, and many people won't understand them at all.

 

The switch statement is specifically for fixed values. But even then you should think twice before you use it, because the fall-through mechanism (all cases up to the next break; statement are executed) make it very error-prone and have lead to a lot of serious defects. The syntax is also rather weird.

 

Use the most readable solution, and that's an if statement in this case.

[mac_gyver]

if you find yourself editing your program logic, just to add, remove, or change values that determine what the logic does, you need to use a data driven design. after your write and test your program logic, you shouldn't be touching the code unless you are adding or changing the features in the code.

 

using the general purpose method 02, in order to change the set-point values, add or remove the quantity of settings, or reuse the program logic (which should be a function/class-method) for a different set/purpose of mapping values, all you have to do is change or supply a different data definition. the data definition(s) can also then be stored in a database where an administrator interface can be provided to easily define/edit the definition(s).

[Jacques1]

The second method is definitely not a good idea, because it relies on implementation details of PHP arrays and can lead to very nasty bugs.

 

For example: It shouldn't make any difference in what order you enter the data, because the CSS classes only depend on the number ranges. So

$data = [
    100 => 'emission_a',
    120 => 'emission_b',
];

should be the same as

$data = [
    120 => 'emission_b',
    100 => 'emission_a',
];

or

$data = [];
$data[120] = 'emission_b';
$data[100] = 'emission_a';

But your implementation does not meet this basic expectation, because you rely on the indexes being ordered. If they aren't ordered (be it because you forgot it, be it because somebody else doesn't fully understand your implementation), the whole thing will fall apart and produce the weirdest results.

 

So, no, you cannot use this. If you insist on this method, you'll have to at least sort the array before you use it. But then again: Is this really the best approach? Sure, it's more flexible, but do you even need this flexibility? Is it worth making the code less readable?