Hacker News Reader

Comment by jedisct1 | Hacker News Reader

All linters and C compilers emit a warning when an assignment is made in a condition.

There are zero reasons to use that ugly and unnatural Yoda notation in 2019.

  if (systemcall(“some string”, expression_argument(args), SC_MODE_1 | SC_MODE_DEFAULT) != 0)
  if (0 != systemcall(“some string”, expression_argument(args), SC_MODE_1 | SC_MODE_DEFAULT))

One may find it easier to read/navigate flow control in C code that returns status codes, when these codes are stated beforehand. When there are series of long lines and a mix of 0==success and 0==false, it is easy to get lost, at least in my experience.

Yeah but that's also just a ridiculously long line which will be hard to read regardless ;)

You either grow it to the right, or in a vertical direction. If the latter, conditions also disperse across that axis. What matters more in a specific case is up to the writer to decide who tries to remain both readable and concise. Highlighting important details in a homogeneous text is part of “the art”.

I'd break this up by lines, every time.

  int result = systemcall(“some string”, expression_argument(args), SC_MODE_1 | SC_MODE_DEFAULT))
  if (result != 0)

Separation of concerns. Each line does one thing. Making a system call and branching on its result are two separate tasks.

By that logic, shouldn't you be creating a var for the expression_argument() return?

Indeed I probably would, just didn't want to distract from the central point in that post.

I would agree with your style, unless all monitors and our vision were built to be more ultrawide-aware than “ultrahigh”. Irregardless of resolution, we can have at most 50-70 lines per eyesight and I doubt that constraint cannot be left unconsidered without losing readability. But it is just my guess from observations, not a real science.

Did you compare writing in different ways and reading it later? This specific style confuses me the most, because I have to mentally connect an assignment with a flow control, because there is no guarantee that a following if() will not use a different variable or it was not shadowed, misnamed, etc. And given that, at once I see x2-3 lines less than usual.

Until you repeat that code pattern 10 times in one function. Then, do you reuse the same result variable? Or have 10 result variables each with a different name? Or put the whole thing in a block to limit that variable to the scope of that block?

In the past, I've done things like this:

  #define TRY(exp) \
     do { \
        int TRY_result = exp; \
        if (TRY_result != 0) \
           return TRY_result; \
     } while (0)

And then instead of the above I can just write:

     TRY(systemcall(“some string”, expression_argument(args), SC_MODE_1 | SC_MODE_DEFAULT)));

(Sometimes, one wants more complex error detection than just comparison to zero, or more complex error handling than just returning the result code. Often, it is possible to build a more complex version of the above TRY macro to meet those specific requirements.)

Furthermore, when we have a problem, if our solution is to have human beings simply remember to do it a different way, we then have two problems.

I find this quote very powerful. I'm thinking I could re-use it. Is it yours or if it's sourced from somewhere could you share the source?

It seems like an intentional reference to this quote (Jamie Zawinski):

"Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems."

LOL That quote is exactly what I was cribbing from. :)

I think this is reasonable readable:

    while ((status = systemcall(...)) != SUCCESS) {
        do something with status;
    }

Leave a comment there explaining the assignment.

Likewise:

    if ((status = systemcall(...)) != SUCCESS) goto error;

Or something like that. I don't understand the hate, just make sure it's obvious what you're doing.

This would achieve the same but is far more readable:

   while(status != SUCCESS) {
      status = syscall(...);
      // do something with status
   }

Only if status is initialised to something other than SUCCESS.

So use a do-while instead.

And this introduces another bug: it'll run the loop body even if syscall fails the first time it's run.

I've been going with this style for things like reading files (with retries) or any sort of loop that feels awkward.

  while (1) {
    int ret = ...;
    if (ret == ...) break;
    if (ret == some_other_condition) break;
    // additional termination conditions....
    // do exactly one thing
  }

Your transformation has introduced a bug: the loop body will run once after success turns false.

It’s traditional to build these kinds of loops in C, e.g.:

    int c;
    while((c = getopt(argc, argv, argstr)) >= 0) {...}

I’d say it’s acceptable, and Python is adding syntax so it can mix assignments and checks.

That's perfectly fine and arguably even better than the alternatives. The `a = b = c = 0;` format is also good.

But enabling it on the language level brings a large amount of risk and complexity just for what amounts to a micro-optimization.

> Leave a comment there explaining the assignment.

Please don't. This is a perfectly understandable idiom in C. Don't explain the language in your comments; that's the job of a text book, which should be read by anyone before they read your code.

Pretty harsh. Imho it's not ugly nor unnatural, just different.

It also reduces time in debugging as you catch it immediately.

Which is great if your project treats warnings as errors. Not sure how common this is though since every time I compile a C program it’s just pages and pages of warnings.

Exactly. All code I write must pass -Wall -Werror -Wextra and there's always a debugging phase once you have written the code since there are syntax errors and whatnot, a lot.

This is true, but sadly warnings can be ignored! At my last job we enforced -Wall so we had no warnings on pull requests to prevent this though.

A useful aspect of yoda notation is that it often makes an equality test easier to read because the constant is shorter than the expression it is tested against, and the == sign comes right away, not buried far away in the line.

It's my favourite way of writing if statements.

Not all programming languages !

I'm so glad somebody else thinks this! I've always hated that style of notation.

Do compilers for other languages emit the same warning? Because there are fewer and fewer reasons to use C in 2019 as well.

Not all languages allows chained assignment (a=b=c) like c++ does, which is why we have this problem (and probably does not automatically cast them to bool either)

Chained assignment isn't the reason. The similarity of the "=" and "==" operators is.

Specifically, it's both (1) easy to make a typo where you meant "==" but typed "=" and (2) not easy to visually distinguish the two.

It is part of the reason. In c++ a=b returns a reference to a to allow chain assignments. And also in c++, most stuff casts to bool automatically when you use them as an if condition. When you have both and make the typo, it gives you a relatively silent bug instead of a compile error. Yea sure, the reason is a typo and easily mistakable operators but the actual reason is the code compiles fine

Java for example expects a bool for for if(). if(a=b) is only valid when they are, which is relatively rare. So it is mostly a compile error.

Yeah, valid point. Those other things are factors.

If C didn't allow assignments to be used as expressions, then it wouldn't be an issue.

Chained assignment isn't the only way to use an assignment within an expression, so I think it's still more accurate not to say chained assignment as the cause, but chained assignment might have been a big part of the motion for making assignments expressions instead of just statements.

Reply on news.ycombinator.com