On Jun 15, 2009, at 4:19 PM, Zach Welch wrote:
On Sat, 2009-06-13 at 21:14 -0400, Duane Ellis wrote:bool okay = *str && !*end && ULLONG_MAX != *ul;In my long career, I have seen too many poor souls - including my self become the victim of even my own seemingly simple attempts to reduce the levels of () and {}. Yes, there are cases where it gets a little toodeep, but there must be a balance.On the opposite side of the coin, I have seen too many programmers relyon extra parentheses because they do not _know_ C operator precedence. There are cases that deserve extra parenthesis, but not on that line.
The extra parentheses and braces do not hurt anything, however. They _do_ make it easier for those who are newer to programming or to clarify the sets of tests being done. I'm personally not fond of incorporating multiple logical (as in separate tests, not specifically those using logic operators) tests into a single statement. If they are going to be, then I prefer the unnecessarily parentheses be used to separate the logical tests to make the statement more understandable.
If removing braces from single-line statements breaks something, then
the statement itself was broken (e.g. a macro lacking do { } while(0)
wrap) or something else needs to be fixed.
While certainly true, it doesn't mean that we _need_ to remove all extra braces. Sometimes they are for clarity rather than functionality.
Openocd is not, and should never become an entry in the 'obfuscated C code contest[1]'. It seems that we are heading down that road. </screaming-rant>Seriously? You think that my efforts have increased the obfuscation? My principle aim has been to improve the readability of the code, so I have to wonder whether others agree with this particular assessment.
I personally find the excerpted line confusing and clunky. I _do_ know what it does, but only because I've written the exact same code to handle the error cases returned by stroul. Without seeing the rest of the code, here's what I don't like about it: - okay is a poor choice of variable name. Its name declares that it is "okay", but it really is telling you _if_ something is okay. is_okay is much clearer. - There are really two separate tests being done sequentially. The first is to see if the whole string was used to generate the number. The second is to see if the parsed number was larger than an unsigned log. That isn't immediately clear even though operator precedence and associativity means it evaluates that way. - The use of 'okay' seems potentially unnecessary. Even if the test is being used to determine multiple control flow paths, the code can usually be changed to only need a single if. The variable declaration doesn't really add anything to the clarity. - If you are testing that a variable is non-NULL, write that. Similarly for a test for NULL. Taking the short-cut just makes it less clear what you are doing and why. Besides, it isn't necessarily guaranteed that NULL will always be 0. ;)
Cheers, Zach _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
-- Rick Altherr kc8...@kc8apf.net"He said he hadn't had a byte in three days. I had a short, so I split it with him."
-- Unsigned
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
