3 ms·

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 lan

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
        Indeed, the code as presented there had well-defined behaviour according to the standard. Note to lunixbochs: This is why I mentioned sequence points before. While valid according to the language definition, assignments within conditional expressions do have a reputation for being error-prone and not always the easiest code to read. Sometimes they're still neater than the available alternatives. However, IMHO in this case, it feels a bit too easy to break the code during later maintenance edits. For example, someone might simplify the code by removing the explicit comparisons with 0 that are unnecessary here, and then later someone else might "fix" an "obvious bug" by replacing the "(err = x) || (err = y)" construction with the common construction "(err == x) || (err == y)" that they assumed was intended, perhaps prompted by a compiler warning. Obviously they shouldn't if they didn't properly understand the code, but when has that ever been a reliable guarantee? :-)

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
      Indeed, the code as presented there had well-defined behaviour according to the standard. Note to lunixbochs: This is why I mentioned sequence points before. While valid according to the language definition, assignments within conditional expressions do have a reputation for being error-prone and not always the easiest code to read. Sometimes they're still neater than the available alternatives. However, IMHO in this case, it feels a bit too easy to break the code during later maintenance edits. For example, someone might simplify the code by removing the explicit comparisons with 0 that are unnecessary here, and then later someone else might "fix" an "obvious bug" by replacing the "(err = x) || (err = y)" construction with the common construction "(err == x) || (err == y)" that they assumed was intended, perhaps prompted by a compiler warning. Obviously they shouldn't if they didn't properly understand the code, but when has that ever been a reliable guarantee? :-)