{"id":441,"date":"2012-01-30T19:26:51","date_gmt":"2012-01-31T03:26:51","guid":{"rendered":"http:\/\/kagan.mactane.org\/blog\/?p=441"},"modified":"2013-06-08T11:14:58","modified_gmt":"2013-06-08T18:14:58","slug":"beware-of-optional-curly-braces-they-will-bite-you","status":"publish","type":"post","link":"https:\/\/kagan.mactane.org\/blog\/2012\/01\/30\/beware-of-optional-curly-braces-they-will-bite-you\/","title":{"rendered":"Beware of Optional Curly Braces — They Will Bite<\/em> You"},"content":{"rendered":"

I was looking through some PHP code from a third-party vendor recently, and saw something that made my jaw drop. It’s pretty innocent-looking, at first. Here’s a somewhat anonymized and genericized version of the code, but the thing that bothered me is still intact. It’s not really a bug, per se<\/i>; the code will function as intended. But…<\/p>\n

$currentRow = 0;
\n$itemId = "";
\n$index = 0;
\nwhile ($row = mysql_fetch_object($result)) {
\n     if ($currentRow == 10) {
\n          renderHeaderRow();
\n          $currentRow = 0;
\n     }
\n     \/\/ takes an itemId and displays relevant columns
\n     renderSummaryRow($row->itemId);
\n     $currentRow++;<\/p>\n

     if ($index > 0)
\n          $itemId .= ","; # interpolate a comma
\n     $itemId .= $row->itemId;
\n     $index++;
\n}<\/p><\/div>\n

See the problem? Really, there are a few ways this can go wrong. To a quick glance, the only clue that line 14 (“interpolate a comma”) is part of a conditional is its indentation. The indentation is important to a human reader — but absolutely irrelevant<\/strong> to the PHP interpreter, which simply treats the next line after the conditional as the conditional’s block. Regardless of how it’s indented, and regardless of what else is around.<\/p>\n

The way it looks to a human is not<\/strong> the way it looks to the machine.<\/em><\/p>\n

What happens if someone wants to add some logging? What if they add it after the comma line?<\/p>\n

if ($index > 0)
\n     $itemId .= ","; # interpolate a comma
\n     writeLog("Added a comma");
\n$itemId .= $row->itemId;
\n$index++;<\/div>\n

Now the log claims the code has added a comma, even when it hasn’t. But still, it could be worse! What if you decided to add your logging before<\/em> the other line?<\/p>\n

if ($index > 0)
\n     writeLog("Adding a comma to itemId...");
\n     $itemId .= ","; # interpolate a comma
\n$itemId .= $row->itemId;
\n$index++;<\/div>\n

Now it adds a comma no matter what — even the first time through the loop, when the string is empty. So instead of a string like '123,124,125'<\/code>, $itemId will now have a leading comma: ',123,124,125'<\/code>. Since this value is getting stitched into a SQL query later on, it means your app will blow up with a SQL syntax error.<\/p>\n

This is why Python makes whitespace significant to program flow. The way the indentation makes it look like<\/em> the logical structure is, is how the structure actually<\/em> is.<\/p>\n

And this is also why Perl — of all languages, one that normally errs on the side of letting you leave out anything that can be inferred from context — Perl<\/strong> insists in its syntax documentation that in cases like this<\/a>, “the curly brackets are required<\/em> — no dangling statements allowed.” (It then says, in typically Perlish fashion: “If you want to write conditionals without curly brackets there are several other ways to do it.”)<\/p>\n

If you’re working in one of those languages that lets you omit curly braces around a single-statement conditional — DON’T DO IT!<\/em><\/strong> The potential maintenance and debugging problems are not<\/em> worth the fun of saving two keystrokes (or just one, if you work in an editor that auto-closes your braces for you).<\/p>\n","protected":false},"excerpt":{"rendered":"

I was looking through some PHP code from a third-party vendor recently, and saw something that made my jaw drop. It’s pretty innocent-looking, at first. Here’s a somewhat anonymized and genericized version of the code, but the thing that bothered me is still intact. It’s not really a bug, per se; the code will function as […]<\/p>\n","protected":false},"author":2,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[1],"tags":[63,7,5,82],"_links":{"self":[{"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/posts\/441"}],"collection":[{"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/users\/2"}],"replies":[{"embeddable":true,"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/comments?post=441"}],"version-history":[{"count":3,"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/posts\/441\/revisions"}],"predecessor-version":[{"id":561,"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/posts\/441\/revisions\/561"}],"wp:attachment":[{"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/media?parent=441"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/categories?post=441"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/kagan.mactane.org\/blog\/wp-json\/wp\/v2\/tags?post=441"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}