This is why you should always use braces on conditionals

In issue 65 on Aura.Router, Optional parameters not working as expected?, we can see that the problem was this piece of code from the issue reporter:

if(isset($this->route->params["id"]))
    echo "id = " . $this->route->params["id"] . "<br><br>";
if(isset($this->route->params["function"]))
    echo "function = " . $this->route->params["function"] . "<br><br>";
if(isset($this->route->params["third"]));
    echo "third = " . $this->route->params["third"] . "<br><br>";

Look at that last if statement: it ends in a semicolon. Although the line beneath it that is *intended* to be the body of the if statement, as indicated by the indentation, it is *not* the body of the if statement. It gets executed no matter the outcome of the if condition.

That kind of thing is really tough to debug.

Instead, if we use braces on all conditionals, regardless of how short or long the body is, we get this:

if(isset($this->route->params["id"])) {
    echo "id = " . $this->route->params["id"] . "<br><br>";
}

if(isset($this->route->params["function"])) {
    echo "function = " . $this->route->params["function"] . "<br><br>";
}

if(isset($this->route->params["third"])) {
}

echo "third = " . $this->route->params["third"] . "<br><br>";

Then it’s very easy to see what’s going wrong.

That is one reason Aura adheres to PSR-2; it incorporates that rule, along with many others, that makes it easier to know what to expect when reading code. Anything unexpected becomes a signal that something may be wrong.

Are you stuck with a legacy PHP application? You should buy my book because it gives you a step-by-step guide to improving your codebase, all while keeping it running the whole time.
Share This!Share on Google+Share on FacebookTweet about this on TwitterShare on RedditShare on LinkedIn

9 thoughts on “This is why you should always use braces on conditionals

  1. nice, my friend mbadolato told me this and since that time I have always adhere to the principle.

    I am in full support of this practice. It just makes sense. It speaks out loud that you are a clean coder.

  2. Braces don’t necessarily help:

    if(isset($this->route->params[“third”])); {
    echo “third = ” . $this->route->params[“third”] . “”;
    }

    • PHP_CodeSniffer can help here. You’ll see it tokenizes that code like this:

      *** START SCOPE MAP ***
      Start scope map at 2:T_IF => if
      *** START SCOPE MAP ***
      Process token 3 []: T_OPEN_PARENTHESIS => (
      * skipping parenthesis *
      Process token 16 []: T_SEMICOLON => ;
      => Found semicolon before scope opener for 2:T_IF, bailing
      *** END SCOPE MAP ***

      So still ends up giving the “Inline control structures are not allowed” error.

      If you use the PHPCBF script in 2.x to auto-fix it, you’ll end up with this code:

      if (isset($this->route->params[“third”])) {
      } {
      echo “third = ” . $this->route->params[“third”] . “”;
      }

      Which is clearly not that helpful, but does match the current execution and makes it much easier to see where the problem is during a code review. If PSR2 had a part of the standard that banned unowned curly braces, then they would have been picked up and removed here as well. Other standards also ban empty IF statements, which would pick up the IF in this case, and might be good in your own custom coding standard.

      Incidentally, the original code ends up being fixed like this:

      if (isset($this->route->params[“third”])) {
      }
      echo “third = ” . $this->route->params[“third”] . “”;

      Again, probably helpful when doing a code review.

    • Braces don’t help, but auto-formatting does. As do compiler checks that don’t allow that stupid construct in the first place.

      • Indeed, Jonathan. This is a misinterpretation of the problem. One has much more to gain by using conditionals in a guard format and fully abstracting class methods.

        Braces can be seen as helpful or as a crutch and I think both perspectives have their place.

        With fully abstracted object oriented code, braces are clutter. With mixed paradigm procedural code they become quite necessary.

        There is really no one right way. Even with object oriented code it can be valuable to adhere to a standard like psr2.

  3. The only time I don’t use braces is when performing a simple one-off if statement, without any other logic. E.g.,

    if ($count > $maxAllowed) $count = $maxAllowed;

    I have no real argument for why except for brevity / LOC. Definitely encourage the use of braces regardless.

  4. Hear, hear! I used to think I was so slick by omitting braces (thank you for not calling them “curly brackets” — they’re braces, dammit!).

    But how many times do you want to go throw a quick debugging output line in the conditional. Well, if you didn’t use braces in the first place, now you have to add them — what a PITA. Just put them there to begin with.

    I’ve never felt like I wasted my time putting the braces in there, but I’ve felt stupid for leaving them off.

    Now python coders — aww, screw them with their stupid indents…

Leave a Reply

Your email address will not be published. Required fields are marked *