thara Posted October 21, 2015 Share Posted October 21, 2015 (edited) 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 Quote Link to comment Share on other sites More sharing options...
Barand Posted October 21, 2015 Share Posted October 21, 2015 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. Quote Link to comment Share on other sites More sharing options...
thara Posted October 21, 2015 Author Share Posted October 21, 2015 (edited) @Barand, Can't we write this using "switch Statement"? Edited October 21, 2015 by thara Quote Link to comment Share on other sites More sharing options...
Ch0cu3r Posted October 21, 2015 Share Posted October 21, 2015 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; } } 1 Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 21, 2015 Share Posted October 21, 2015 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. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted October 21, 2015 Share Posted October 21, 2015 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). Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 21, 2015 Share Posted October 21, 2015 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? 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.