[ros-dev] [ros-diffs] [dgorbachev] 39422: Fix bug #4129 in CmpGetNameControlBlock().

[Timo Kreuzer]

Zachary Gorden schrieb:
> I agree with the requirement for the parenthesis, simply because it helps
> separate the various conditions that are being tested for.  Even looking at
>   
The conditions are clearly seperated by || or by &&. It should be
instantly clear that the comparison is higher than || and && in the
order of operations.
Something like "if (a == (b || c) > d)" wouldn't make sense anyway.

Do not mess this up with bracing things like
if ((a == 0 || a > 2) && b < 0)
Although the braces are not needed, it makes sense to set them.


> the code without braces, you should be questioning exactly what is going on
> in there since you generally don't do assignments inside if statements.  The
>   
You will probably find a lot of "if (!(ret = FooBar()) error();" here
and there in our code.
Maybe we should have a rule that you *must not* do that.

> fact that the compiler failed to catch it because of the parenthesis doesn't
> excuse a programmer from not seeing the problem.  I personally would never
> assume that a dev intended to do an assignment just because he or she put
> parenthesis around such a statement, I would automatically assume they
> messed up typing and left off a = in the check.
>   
It's of cause the programmers fault.
That doesn't make forcing a coding style that hides such bugs any better.
This kind of error is quite common. The proposed rule will effectively
prevent these typos from being detected when compiling your code.


I would agree on the rule if we could make the compiler always scream
when someone does an assignment inside an if statement and we would
prohibit this totally.