6 ms·
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 20
by jedisct1 7y ago
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.
- codegladiator 7y agoNot all programming languages !
- na85 7y agoDo compilers for other languages emit the same warning? Because there are fewer and fewer reasons to use C in 2019 as well.
- shultays 7y agoNot 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)
- adrianmonk 7y agoChained 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.
- shultays 7y agoIt 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.
- adrianmonk 7y agoYeah, 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.
- wruza 7y agoif (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.
- admax88q 7y agoYeah but that's also just a ridiculously long line which will be hard to read regardless ;)
- wruza 7y agoYou 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”.
- T-hawk 7y agoI'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.
- saw-lau 7y agoI'm so glad somebody else thinks this! I've always hated that style of notation.
- roland35 7y agoThis 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.
- deleted 7y ago[deleted]
- beatgammit 7y agoI 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.
- Sahhaese 7y agoThis would achieve the same but is far more readable: while(status != SUCCESS) { status = syscall(...); // do something with status }
- taneq 7y agoOnly if status is initialised to something other than SUCCESS.
- blktiger 7y agoSo use a do-while instead.
- Izkata 7y agoAnd this introduces another bug: it'll run the loop body even if syscall fails the first time it's run.
- dpzmick 7y agoI'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 }
- Izkata 7y agoYour transformation has introduced a bug: the loop body will run once after success turns false.
- deleted 7y ago[deleted]
- johnfactorial 7y agoFurthermore, 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.
- muench 7y agoI 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?
- NoodleIncident 7y agoIt 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."
- deleted 7y ago[deleted]
- johnfactorial 7y agoLOL That quote is exactly what I was cribbing from. :)
- deleted 7y ago[deleted]
- weberc2 7y agoWhich 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.
- emilfihlman 7y agoExactly. 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.
- ur-whale 7y agoA 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.
- emilfihlman 7y agoPretty harsh. Imho it's not ugly nor unnatural, just different. It also reduces time in debugging as you catch it immediately.