Incidentally, I found a cool way of checking whether NaN occurred:
Just do:
void some_function(float number)
{
if(number != number)
number was passed in as <NaN>
}
Took me a little bit to get my head around it but if the number is not equal to the number, then a problem occurred. I used this to stop camera code (driven by floats) from crashing when receiving non-sense input coordinates. It still doesn't work properly, but it doesn't crash now either! (at least not for that :)
Whenever the whole comments argument comes up I like to bring up this article[0]. I find that it describes a good balance between when comments are useful and when they are not.
/begin rant
I'm working on a C++ code base developed by contractors that were lazy and doing things the expedient way instead of the correct way (thousands of circular dependencies between libs - and even apps - apps depending on source from other apps), multiple copies basic functionality with slight changes (largely bug fixed in one place, but they forgot the other X places it was copied).
I've gone so far as to write a "cleanup" script in python that runs a number of transformations on the source. Fixing things like inconsistent line-endings (via dos2unix), inconsistent formatting (via clang-formatting), limited conversions of certain Boost uses to std C++, a few pervasive spelling errors (contracts were eastern European, non-native English speakers). Another thing I'm considering is removing all comments. Lots of dead, commented out, code. Also, invalid UTF-8 characters in comments, from an ANSI code page I've not been able to identify (which of course creates problems with the python script treating the source as UTF-8).
The comments that are currently present that I've seen are:
1. stupid/moronic/redundant (like merely marking constructors as ctor and destructors as dctor with no more info - like I can't just figure that out by reading the source to begin with)
2. Just plain wrong. As-in, comment no longer matches the code.
3. In a foreign language (Ukrainian/Russian), in an unknown code-page, so also worthless at face value (yeah, I could lookup the code pages for Ukraine and Russia have give it a shot and run the result through a translator), but it's likely the result will also fall under #2.
4. Unnecessary demarcation between functions. e.g. "// -------" out to ~80 characters wide between functions with no other white-space. Want things to be clearer? Don't use K&R notation and but open & close braces on the same indentation level (e.g. don't put the opening brace at the end of your loop/function/if/else/other statement, but put them alone on the next line).
5. Dead code. Don't commit commented-out code. Delete it. That's what version control is for. Want to know what it used to do? Look at the history, not the chain of commented-out code. Commenting out code is fine for quick testing. But don't commit that. People that follow you will see that and wonder "why is this here? is this significant?"
6. Random BOM (byte-order-marks) for UTF-8 source that is actually entirely ASCII. A lot of linux tools don't expect or handle BOM in UTF-8 sources (I'm looking at you, psql).
In this particular project I'm working on at my employer, I don't think I've seen a single useful comment in the source. Sadly, the Jira tasks filed by the now-fired contractors are also nearly universally useless. All headline and mostly zero description on what the problem is, usually no steps to reproduce.
/rant
Sorry for the rant, when I started writing this, I did not intend it to be so. But, as I wrote, I started remembering more and more things that have been driving me nuts.
In one project I inherited, some contractors had been maintaining the code for about a year. Every change they made they added a comment box around it like "Mike made this change on Dec 31". What's worse was they included a copyright indication (I tried to find out if anyone had gotten a copyright assignment from them and the legal department literally slammed the door in my face). Their changes were bizarre and included a lot of things you ranted about. I reverted the code to the state before they arrived and sent the result to QA. "Wow! How did you fix so many bugs so quickly?" was the response from QA.
We had an iron clad contract with that contracting firm, but after I started reverting every single subsequent change they made, they stopped sending changes ;-) Luckily my boss was a director and could pave the political fallout from my rather brash (but justified) actions.
Comments need to explain why they are doing something, not what the code is doing. I can tell what the code is doing by reading the source. Otherwise, I agree. The code should be clear enough that no comments are needed.
Well, good code obviates the need for most comments. A comment shouldn't cover up a bug of clarity in a piece of code, in almost any case imaginable (basically, if it isn't a Fast InvSqrt() level of utility, the dev ought to reconsider the "cleverness" of the code). And anyhow, the implementation details of the code are liable to change; the comment becomes outdated, and there's a good chance that it won't be updated.
What about a comment that notes what part of a spec some code implements (i.e. something outside the actual behavior of the code)? A comment that answers "why" can be helpful, and can sometimes be worth the high cost that an unchecked, unexecuted part of the program inherently carries.
If Fowler's claiming that all comments are bugs, I'd call that damaging.
// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
// qualified by an address-space qualifier."
if (Type->isFunctionType()) {
S.Diag(Attr.getLoc(), diag::err_attribute_address_function_type);
Attr.setInvalid();
return;
}
The comment justifies the code in a way that the code itself never could.
A unit test could document that equally well (although the pointer to the spec would still be useful). Whether or not it would fit your style of programming is another matter, of course.
Maybe I'm missing something. The code that you've written doesn't look any clearer to me than the code from the compiler itself, and without the comment, there's no "why" for the behavior.
Later, if a bug is filed saying that the compiler isn't compliant with the requirements of "ISO/IEC TR 18037 S5.3", how can you be sure to find the code where the behavior is implemented?
I would put that comment in the function header comment, not in the code itself. That's more meta.
And of course the function in question only deals with "ISO/IEC TR 18037 S5.3" so it's easily tested.
If someone files a bug saying the compiler isn't compliant with the requirements of "ISO/IEC TR 18037 S5.3", then they would provide a test case showing not compatible. Add that case to the existing unit tests and you will see which function fails. No need for searching the code to see where the behaviour is implemented. With clean code it's obvious, the test will show this method to be at fault. Even without any comments.
Code in functions should also try and stay at the same level of abstraction, moving low level stuff to abstracted methods that describe the intention (And then the low level method does the how). That way your code reads like a story.
> No need for searching the code to see where the behaviour is implemented. With clean code it's obvious, the test will show this method to be at fault. Even without any comments.
So...it sounds like you'd have one broken implementation of the code, and one working implementation, that might just cancel out the effects of the broken one. You're assuming code that has been cleanly written for its whole history, or a lot of love poured into it to develop quality tests for each requirement. How commonly does that actually happen?
That wasn't the most even-keeled response, and it's past the edit window. What I mean to say is that I've never seen anything but a small codebase that couldn't use some in-code explication.
Clear code provides a clear "how". Good test coverage can act as documentation of the proper behavior and help prevent regressions. But I don't see how it follows that clean code makes the location of each implemented feature obvious. It doesn't seem like it would be inherently true.
But isnan is the least fancy way to check if something is NAN. What you're doing is fancy. I mean, that's exactly why you said "it took me a little while to get my head around it."
Be careful though when compiling your code with -ffinite-math-only (which is part of -ffast-math and -Ofast). The compiler will likely replace your function with a constant expression returning false, since it assumes that NaNs cannot occur in your code. I found that the only safe way to actually check for NaNs when using these flags is to memcpy the float into a byte array (to avoid aliasing issues) and manually compare against the NaN bitmask.
I wouldn't be surprised if the future compilers would optimize it away as well, since ffast-math basicly makes NaN's equivalent to undefined behaviour.
Just do:
void some_function(float number) { if(number != number) number was passed in as <NaN> }
Took me a little bit to get my head around it but if the number is not equal to the number, then a problem occurred. I used this to stop camera code (driven by floats) from crashing when receiving non-sense input coordinates. It still doesn't work properly, but it doesn't crash now either! (at least not for that :)