What is the best way to structure conditional logic?

Question

It occurs to me that there are number of different ways to structure conditional logic. As far as I can see, as long as we set errors to end the script (or you can imagine the same examples but with a return in a function), then the following examples are equal:

Example 1

if($condition1) {
    trigger_error("The script is now terminated");
    }

if($condition2) {
    trigger_error("The script is now terminated");
    }

echo "If either condition was true, we won't see this printed";

Example 2

if(!$condition1) {
    if(!$condition2) {
        echo "If either condition was true, we won't see this printed";
        }
    else {
        trigger_error("The script is now terminated");
        }
    }
else {
    trigger_error("The script is now terminated");
    }

Example 3

if($condition1) {
    trigger_error("The script is now terminated");
    }
else {
    if($condition2) {
        trigger_error("The script is now terminated");
        }
    else {
        echo "If either condition was true, we won't see this printed";
        }
    }

Example 4 -- Adapted from Fraser's Answer

function test($condition) { 
    if($condition) {
        trigger_error("The script is now terminated");
        }   
    }

test($condition1);

test($condition2);

echo "If either condition was true, we won't see this printed";

Personally, I lean towards writing code as in Example 1. This is because I feel that by checking for conditions that end the script (or function) in this way, I can clearly define what the script executed and not executed i.e. everything before the condition has been executed and everything after the line has not. This means when I get an error on line 147, I know immediately what has happened helping me to find a bug faster. Furthermore, if I suddenly realise I need to test $condition2 before $condition1, I can make a change by a simple copy paste.

I see a lot of code written like in Example 2 but for me, this seems much more complex to debug. This is because, when the nesting gets too great, an error will get fired off at some distant line at the bottom and be separated from the condition that caused it by a huge chunk of nested code. Additionally, altering the conditional sequence can be a lot messier.

You could hybrid the two styles, such as in Example 3, but this then seems to overcomplicate matters because all of the 'else's are essentially redundant.

Am I missing something? What is the best way to structure my conditional code? Is there a better way than these examples? Are there specific situations under which one style may be superior to another?

Edit: Example 4 looks quite interesting and is not something I had considered. You could also pass in an error message as a second parameter.

Thanks!

P.S. Please keep in mind that I might need to do some arbitrary steps inbetween checking $condition1 and $condition2 so any alternatives must accommodate that. Otherwise, there are trivially better alternatives such as if($condition1 || $condition2).

Answer 1

#1 is by far the clearest. However, if somehow the thing that previously ended the execution were changed to do something else, then it would break.

It's still probably best to go with #1, but make sure the thing being used to "stop" is clearly named to indicate that it does stop things, so that someone in 10 years maintaining your code doesn't accidentally break things by changing it.

Answer 2

I think your method (example 1) is the most efficient and effective in this type of situation. However, there are times when you do not want any conditions to halt execution and you only want to execute condition2 if condition1 is false. In these situations, an else or elseif works well.

Answer 3

I prefer this style, which doesn't break if either of the conditional blocks are changed so they do not exit execution.

if($condition1) {
    trigger_error("The script is now terminated");
}
if($condition2) {
    trigger_error("The script is now terminated");
}

if (!$condition1 && !$condition2) {
  echo "If either condition was true, we won't see this printed"; 
}

Edit: Missed the PS, so updated code to match the full question details.

Answer 4

I am in the Example 1 camp. As a rule of thumb, the less indentation needed, the better.

// Exit early if there are errors.
if ($n < 0) {
    die "bad n: $n";
}

// Handle trivial cases without fuss.
if ($n == 0) {
    return 0;
}

/* Now the meat of the function. */
$widget->frob($n);
foreach ($widget->blaxes as $blax) {
    checkFrobbingStatus($blax);
}
// ...And another 20 lines of code.

When you use an if/else and put the success and error code in parallel sections you make it appear as if the two code blocks are equal. In reality, the edge cases and error conditions should be de-emphasized. By intentionally handling errors early and then not putting the "important" code in an else clause I feel like that makes it visually clearer where the important code is.

"Here are all the preconditions. And now here's the good stuff."

Answer 5

I suggest using ‘try‘ clause over any part that can have errors and use ‘throw "error description"‘ each time an error occures(like example #1).
That way you can have error reporting code once in your program (in the ‘catch‘ clause) and splitting code into functions won't be a hussle rewriting error handlig.

Answer 6

I generally agree with Amber insofar as your first option seems the most legible. This is something I have fought with myself - thus far the only reasoning I have stumbled across is as follows:

• The first form is clearest when reading through a linear script, so ideal for simple scripts
• The second form is cleanest when you need to ensure tidy / clean-up operations

I mention the second because this is a sticky point. Each script may be part of a larger system, and in fact the script elements you are injecting the "bail out" code into may be called by multiple places. Throw in some OO and you've got a real potential pickle.

The best rule of thumb I can recommend is that if your scripts are simple and linear, or your are doing rapid prototyping, then you want to use the first form and just kill the execution at that point. Anything more complicated or "enterprise-esque" will benefit from (at least) a modular redesign so you can isolate the method and the call stack - and possibly encapsulation of an OO build.

With some of the more powerful debugging and tracing tools which are available these days, it is becoming more a matter of personal style than necessity. One other option you might consider is to put information in comments before (and possibly after) each bail-out zone which make it clear what the alternative is should the criteria be met (or failed).

Edit:

I'd say Fraser's answer is the cleanest for encapsulation. The only thing I would add it that you might benefit from passing an object or hash array into the standard "bail out, I'm dead" method so you can modify the information made available to the function without changing parameter lists all the time (very annoying...).

That said - be careful in production systems where you may need to clean up resources in an intermediate state.

Answer 7

Personally I hate nested if-else statements so #1 for me from your examples. The other option I would look at is something like the following.

function test($condition) { 
  if($condition) {
    trigger_error("The script is now terminated");
  }   
}

test($condition1);

//do stuff...

test($condition2);

//passed the tests

EDIT: The more I think about it a functional approach is by far the best way in that it negates having to write the logic that tests the conditions more than once. It also allows greater readability because it is obvious you are 'testing' the condition (as long as you give the function a meaningful name). Also, as pointed out in the question edit it would be trivial to pass other parameters to the function. i.e.

function test($c, $msg) { 
  if($c) {
    trigger_error($msg);
  }   
}

test($condition1, "condition1 error");
test($condition2, "condition2 error");

Answer 8

The best way to structure conditional logic is to follow the logic itself.

If you have dependencies, say, failing of first condition will make others unnecessary is one thing. Then return, goto, nested conditions and exceptions at your choice.

If you are about to make decision upon a test, say

if (!isset($_GET['id'])) { 
  //listing part: 
} else { 
  // form displaying part: 
}  

it's else, elseif and case realm.

etc.
Determine your program logic first and write it's logic then.

and trigger_error() has nothing to do with conditions. It is debugging feature, not program logic related one.

Answer 9

I much prefer #1 as well.

In addition, I really like to assign variables during the conditionals

e.g.

if ( !$userName = $user->login() ) {
    die('could not log in');
}

echo "Welcome, $username";

I usually find that in the first write of code, I end up with a fair few messy nested conditional statements, so it's usually during a second pass that I go back and clean things up, un-nest as many of the conditionals that I can.

In addition to looking much neater, I find it conceptually much easier to understand code where you don't have to mentally keep track of branching logic.

For branching logic that can't be removed that contains lots of procedural code, I usually end up putting it in a function / class method -- ideally so I can see on one screen all the branching logic that is taking place, but modifying either the actions or the logic won't break the other one.

Answer 10

Example 5

if($condition1 || $condition2)
{
    echo "If either condition was true, we won't see this printed";
}else
{
    trigger_error("The script is now terminated");
}

Example 6

function allAreTrue()
{
    foreach(func_get_args() as $check)
    {
       if(!$check)
       {
           return false;
       }
    }
    return true;
}

if(allAreTrue(true,true,$condition1,$condition2,false))
{
   exit("Invalid Arguments");
}

//Continue