Whoa, that's quite a long list!
All right, I accept the challenge

.
Btw, it would be nice if you conceed a point every now and then

.
D H said:
Just as a starter, it's 71 pages long. This puts the emphasis on the wrong syllable. Coding represents 10% or less of the effort in a safety critical project. Remember rule #0: Don't sweat the small stuff.
I disagree. In large scale software development, every bug that has to be found and solved is a pain in the ***, if only due to the overhead. If a couple of bugs can be prevented, that is worthwhile.
I know there are various studies that relate bugs in code to overhead and more importantly to catastrophic failures in practice.
And obviously NASA thinks it's important. I found the following quote:
"The NASA has a defect density of 0.004 bugs/KLOC (vs 5.00 bugs/KLOC for the Industry) but this has a cost of $850.0/LOC (vs $5.0/LOC for the Industry). Source: agileindia.org/agilecoimbatore07/presentations/… – Pascal Thivent Sep 21 '09 at 21:43"
D H said:
Rule 2.1. Comments in the code are not the place to document waivers, nor are they place to document significant design decisions. That said, this is the *only* rule on comments? Please.
Agreed, although this only means that you need additional coding guidelines.
D H said:
Rule 3.1.1. This is a good rule in general except for the fact that C++ (stupidly) provides, a gratis, a default constructor, a copy constructor and an assignment operator. It is a common practice to declare the copy constructor and assignment operator private and, oops, forgot to implement them. Fortunately C++0x will fix this gaping hole in the language. Until then, I want to see that the programmer has used this trick right up front.
Yes, this is a good rule.
As for the gaping hole in the language, it is addressed a little further in the standard (Guideline 3.1.13), that is, the copy constructor and assignment operator are required to be defined. We use a template that defines them private (and not implemented) by default. And we use a static analysis tool that warns if they're missing.
D H said:
Rule 3.1.4. Requirements should never specify an implementation. Ever.
Actually, this is a bad rule for defining a copy-constructor, because of the implications it has that are not addressed.
However 3.1.5 (exclusive with 3.1.4) is a good rule. I've already seen too many stupid bugs because this rule was not observed.
And note that it is a check list, not an enforcing implementation format.
D H said:
Rule 3.1.6. This rule, like many of the rules in this standard, is chasing compiler problems that were solved in the previous millennium. It's past time to join the 21st century.
How were they solved (inline virtual functions)?
D H said:
Guideline 3.1.7. Placing function definitions in the body of the class detracts from understanding the class. Even a simple three line function expands to ten lines or more if one properly documents the function such as for processing by doxygen. It is far better to have a one line comment and a prototype in the class body and to inline the function outside the body, preferably in a separate inline header file.
No argument there, although I don't like long inline functions in header files.
D H said:
Guideline 3.1.9. Even a half-assed code review will detect duplicated code. All it takes is a reviewer to say "this code is duplicated in three places. That duplication will cost a lot of money in documentation, testing, verification, and validation." A half-assed code review will catch many of the problems mentioned in these standards. A good code review process will catch many, many more problems. Having humans evaluate a chunk of code still stands as the #1 way to catch bugs. Testing, V&V, and IV&V follow. Meeting or violating coding standards is way down on the list. It is best to keep the coding standards simple and stupid. This is one of many violations of that metarule.
I think you're agreeing with the Guideline?
Note that the Coding Standard does not say how the rules are verified, although I think it's a good thing to have a static analysis tool generate warnings for duplication.
D H said:
Guideline 3.1.12. Pretty much useless in scientific programming (what, exactly, should << to an ostream do for a 300x300 spherical harmonics gravity model?), and downright stupid in an environment that says thou shalt not use C++ io (or C io). In some cases a related rule is useful (thou shalt provide a serialization/deserialization capability for all data), but that is rather special purpose.
I agree, but this is a "Guideline", which means it is not required unless the programmers in your firm want it.
D H said:
Rule 3.2.2. This is a part of the C++ standard itself, and a short falling that one day will be rectified. Why repeat it?
If members are initialised in the constructor using other members, this leads to bugs, because the order may be different than you expect. On other words, some members may end up uninitialised.
Anyway, it bugs me no end if the order of members is haphazard and unpredictable.
D H said:
Rule 3.3.5. Have these guys not heard of the using clause? Besides, a simple rule, "Thou shalt compile clean with <project-specific> compiler settings", will catch this problem and a whole lot more.
I don't understand.
I you overrule one version of a virtual method, it's weird if you don't overrule all versions of that same virtual method.
D H said:
Rule 4.1. A rule on complexity is good. A limit of ten is not good. This rule of ten is a cargo cult rule with zero backing in practice. Multiple studies have shown that this limit is too low, that it in fact leads to buggier code. NASA IV&V uses 20 as a limit. There are occasions when an incredibly high complexity is perfectly OK. I have blessed (granted a waiver to) a function with a complexity greater than 500.
I can predict the reason for your waiver: it's a switch-statement isn't it?
I believe there should be an exception for a switch-statement.
Even better would be a separate rule for switch-statements.
I believe it's a bad thing if many methods in your class have the same switch-statement. It means the design of the class is flawed.
I agree that 10 is too low, but as you said, your company is free to change or discard any of the rules in the standard. It's enough that such is stated in a quality assurance manual.
D H said:
Rule 4.2. Static path count is one of many, many metrics that has a relatively low correlation with bugginess, quality, understandability, etc. Even worse, what it does measure is better measured by cyclomatic complexity (apparently even better is extended cylcomatic complexity). What about the #1 metric that correlates with bugginess? Not a mention. It's too trivial. After all these years of various people developing various metrics, nothing tops lines of code. Nothing.
Agreed.
There should be a rule in here that says the the lines of code of a function should not exceed some number, instead of this one.
D H said:
Guideline 5.7. This flies in the face of recommended practice for iterators.
This rule is indeed too indiscriminate.
But I do believe that a loop-condition should not be an unnecessarily complex expression which causes an unnecessary performance penalty.
D H said:
Rule 5.9. For one thing, it looks to me like this rule allows the use of Duff's device. For another, the single point of entry / single point of exit rule is responsible for a lot accidental complexity, maybe more so than any other ill-considered, cargo cult rule.
I agree, although almost all of my colleague's don't.
I do believe it's best if all programmers in a team use the same structuring for a function, so I've stopped arguing about this.
Btw, I found the reference to Duff's device interesting reading material.
D H said:
Rule 8.4.9. Sorry, STL vectors do not cut it when I want a 3-vector. Or a 3x3 matrix. Amazingly enough, 3 vectors and 3x3 matrices pop up quite often in my work. A lot more than does the need for that misnamed std::vector (it is not a vector).
A 3-vector or 3x3 matrix is NOT an unbounded array.
And btw, a vector does *not* enforce array bounds.
I do believe that for arrays of variable or large length, vector really is the best choice.
D H said:
Rule 10.1. Like grep, I too have seen fools mandate the use of #define TWO 2. Not using magic numbers is a good guideline. When specified as a rule, it is the fools who rule.
Have you ever seen a function call like:
some_function(2, 3, true, false, false);
If I read something like that I think: what the **.
And it doesn't becomes better with the use of TWO ;-).
I agree that a warning on '2' in a mathematical formula is probably a false positive.
D H said:
Rule 10.3. Yet another rule that is explicit in the standard. The example is particularly bad. Can't we just say "thou shalt not depend on undefined behavior"?
Ah, but we still need a list of all the "undefined behaviors" there are.
The coding standard is a good place to put that list.
D H said:
Rule 10.5. This rules out using a=b=c=0;
I see the mathematical algorithms popping up again.
That's where you (want to) use that, right?
For mathematical algorithms I agree.
In my experience in other cases you don't really need this, and in general the code is easier to read if declarations and initialisations are on different lines.
D H said:
Guideline 10.6. This is one of the dumbest ideas ever. It looks like the authors of this standard developed rules by browsing every standard they could find.
Better safe than sorry.
What's wrong with coding guidelines that make sure a programmer can't make a mistake even if he wanted to?
And yes, I believe they browsed every standard they could find and tried to put at least the most useful rules in.
But knowing programmers, each and every one (including myself), disagrees with any number of rules.
I still think it's a good idea that the programmers in a team use a common set of coding guidelines (even if they disagree with them).
D H said:
Rule 10.10. Whatever the true intent of this rule is, what they said makes no sense. (And it flies in the face of some other languages whose goal is no side-effects.)
Basically it says: do not write redundant code.
The term "side-effect" may be misleading here.
It's a statement that doesn't "do" anything.
D H said:
That's a bit more than half-way through this work of art whose sole aim is to sell a product, and I did not hit all of the objectionable points. Do I really need to continue?
I see you have skipped a lot of the really useful rules.
Can it be you agree with a number of them?
Note that any rule can be changed or discarded.
It's enough that such is stated in a quality assurance manual (which is required).