3 ms·

Braces don't help in coding styles that put the opening brace on a separate line from the control statement. if ((err = SSLHashSHA1.update(&hashCtx, &signedP

• bunderbunder 13y ago
  If you have a programmer on your team who is likely to modify if (condition) { doSomething(); } into if (condition) { doSomething(); } { doAnotherThing(); } instead of if (condition) { doSomething(); doAnotherThing(); } then that person needs some serious mentoring right away. 'Cuz. . . just wow. As far as the original example goes, if it's an error it's most likely a copy/paste error. Curly braces help there, too. With three times as many lines in the block, the odds of a paste error resulting in code that even compiles is greatly reduced, and a compiler error should call attention to the issue. Even assuming the bug was malicious, the curly braces would increase the odds of it being caught by another developer. This is particularly the case now that offside rule languages are common. A large chunk of younger devs cut their teeth on languages where if (condition) doSomething(); doAnotherThing(); doesn't look odd in the slightest. But I think that the 2nd example above would still look immediately bizarre to nearly everyone.
  • orangecat 13y ago
    As far as the original example goes, if it's an error it's most likely a copy/paste error. Right, and this demonstrates the major problem with verbosity in languages and APIs and design patterns. When you have to repeat yourself many times, it's very easy to make a mistake in one of the near-copies, and you or a code reviewer can miss it because you'll tend to quickly skim over the boilerplate. For cases like this, using exceptions rather than manual error value checking would make your code shorter, less redundant, and not susceptible to this particular bug. (Not possible in C, I know).
    • bunderbunder 13y ago
      Not possible in C, and I'm not even certain that high-level exceptions are desirable in a language like C. But I wonder if there's still room to tighten up the code. Perhaps something like if ( (err = SSLHashSHA1.update(&hashCtx, &serverRandom) != 0) || (err = SSLHashSHA1.update(&hashCtx, &signedParams) != 0) || (err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)) { goto fail; }
      • Silhouette 13y ago
        I understand that there are sequence points at those || divisions, and I appreciate that you've been careful with your layout. Even so, my spider sense is tingling horribly at having not just one assign-and-test in the condition for an if statement but a whole series of them that each reassign to the same variable. If there were a language feature available to do this more elegantly, whether exceptions or something else such as a Haskell-style monad with fail semantics, I'd almost certainly agree with using it in preference to a series of manual checks, though.
        • lunixbochs 13y ago
          Should the variable reuse matter? Is there a case where the compiler won't short-circuit the || statement on the first failure? If not, `|| (err = check())` is equivalent to separate checks which also also `goto fail` immediately.
          • bunderbunder 13y ago
            >> Is there a case where the compiler won't short-circuit the || statement on the first failure? Left-to-right, short-circuit evaluation of the logical operators is so deeply rooted in C idiom that a mainstream compiler that doesn't respect it would be practically unusable. But perhaps it wouldn't work in a hobbyist compiler, or a non-standard dialect of C.
            • Silhouette 13y ago
• lucian1900 13y ago
  That is a lot more obviously wrong that without the braces, though. But yes, mandatory braces on the same line is the correct choice.
• pacaro 13y ago
  Another solution, (popular at Microsoft FWIW) if you have a common pattern like if ((err = DSomething()) != 0) goto fail; is to wrap it in a macro