coding practices

[ischuldt]

How would you rank the following coding practices in order of most important to achieve in writing a c++(or any other) method(1 being most important)?

1.)Minimal code length. Trying to keep all code for a method on one screen.
2.)Minimal indention. Start every line of code as far to the left as possible.
3.)Minimum try blocks. Try to keep your try blocks so they only contain the statement that throws the exception.
4.)Minimal Exit points. Try for a single return.
5.)Minimize Negation. Say if(x && Y){doA}else{doB} rather than if(!x || !y){doB}else{doA}
6.)Normal flow to the left. Keep the expected and most likely path through the code as far to the left as possible.

Feel free to add in order any other coding pet peeves that you hate seeing.

[Galik]

1) Different people have different size screens. Just make the code tidy and readable and not too wide (80 chars max?).

2) Use consistent indentation. Always indent a block.

3) No. If you are going to do that you would be better checking a return code and forgetting about exceptions.

4) Yes. Great to do this if practical! Sometimes it makes sense to have more than one return. But avoid it if possible..

5) Keep logic as obvious as possible. Sure. But sometimes negation is more obvious: while(!done){}

6) ?? Left? I don't understand!

[Athar]

4. mostly applies to C programs, where resource cleanup is not automatic. Therefore multiple return points can mean a lot of duplicate cleanup code and it carries a high risk of forgetting to cleanup something at some point. In C++ when following RAII there is no danger of that, so returning early is fine if it helps readability (and it often does).

This is also related to 6... it's generally better to avoid deeply nested blocks. So it's usually better to write

if (!condition)return;
[...]
//determine whether next condition is true
[...]
if (!conditon2)return;
[...]
//do what you wanted to do after possibly checking some more conditions

//...instead of:
if (condition)
{
  [...]
  //determine whether next condition is true
  [...]
  if (condition2)
  {
     [...]
  }
}

With return (or break in loops) you know that everything ends right at that point, whereas deeply nested if blocks can get confusing.

Agreeing with Galik on the other points.

[kbw]

5) I disagree with here. Remember order of evaluation is important, so x && y evaluates both x and y, whereas !x || !y only evaluates y if x is true.

[Athar]

x && y also only evaluates y if x is true...

[kbw]

The example is bad as the two statements are not equivalent. I missed that.

If we tweak the example so the two statements are logically equivalent by DeMorgan's rule, we have !(x && y) and (!x || !y).

The first may apeal to sense of readability, but is more inefficent.

[jsmith]

Agree with (1) because short functions are easy to understand and less likely to have bugs.

Use a reasonable tab stop. Most people start to complain if the tab depth isn't at least 3, but
tab stops that are too wide mean a lot of line wrapping.

If I understand (3) it doesn't really matter. Perhaps for the sake of self-documenting code
you should limit the try block to only that which can throw, but I wouldn't go out of my way
to ensure that. ie, try { some_function_that_throws(); ++x; another_function_that_throws(); }
is fine even if ++x can't throw.

(4) Agree with Athar. C++ is designed for early exit, but it can require RAII to do correctly
(you don't want to have to write:

if( !some_function() ) {  // failure case
    do_a_ton_of_cleanup_here();
    return;
}

Ideally just

if( !some_function() )
    return;

and let destructors do the cleanup.

5) More generally, minimize logic, unless it is less understandable that way.

6) Agree, though if that means more code, I might break the rule. (What OP is saying is
that the coding technique used for the function should be like:

void foo() {
    if( some_bad_condition_1 ) {
       // error code here
    }
 
    // do work in success case

    if( some_bad_condition_2 ) {
       // error code here
    }
 
    // do more work in success case
}

That is, the "success" case of the function should lead you to the end of the function.
Or, said another way, all early exits are error cases.

[Galik]

With respect to (4) clean-up is not the only issue. The other thing we need to be careful of is the return value. It can lead to maintenance mistakes if you decide to modify the return value in some way. When there are many exits you may need to visit each exit to perform the modification. That could lead to one exit being forgotten.

But with all these 'rules' there are always exceptions and times when the rule to apply is different. The 'ideal' is to understand the various trade-offs and use the most appropriate solution for each situation. That, however, is something we will never stop learning.

[PanGalactic]

#1 is the only one I agree with, with the caveat that you are not sacrificing readability to achieve this, but are instead further breaking the problem down into smaller chunks. A natural consequence of this is that indentation level stays low. Both length and indentation levels are indications to me that it is time to start a new function.

I've seen people make their code do odd contortions in order to fit such simple rules without understanding why such rules exists.

I strongly disagree with #4. To quote Alexandrescu:

Such a requirement is obsolete in a language that supports exceptions and destructors, where functions typically have numerous implicit exits.

Keep the code short and it is my belief that Galik's concern about multiple returns disappears.

I try to keep function lengths under 24 lines unless doing otherwise increases complexity and decreases testability.

[ischuldt]

One thing of note. The fact that I accidentally wrote the inverse of my x && y statement wrong and at first nobody caught it would seem to be further proof that all the ! and || tend to be hard to read.

Also I wasn't really asking whether or not you should try and do any of these things as much as which one's are the most important. If you can't acheive all of them, or you have to sacrifice one for another which would you choose.

[Galik]

To be honest I think you apply as many 'good' practices that you know, all the time. When there is a trade off between two or more then the situation should dictate which is more appropriate.

I couldn't say that one is more important than the other, because when I recognise both, I apply both. Unless there is a trade off, in which case the circumstances will force a decision.

[Kyon]

I most definitely do not agree with 1. Away goes the meaning of prototypes. Less code =/= more readability. In a class definition or in your global scope, try making a list of prototypes, which you later define underneath the main function. Ensure they work correctly and have logical names and return and input types and THAT will increase readability by a LOT.

[Athar]

ischuldt wrote:

as which one's are the most important. If you can't acheive all of them, or you have to sacrifice one for another which would you choose.

The replies should have told you that none of these rules are important and that you shouldn't try to achieve any of them.
Do what makes sense in the case at hand, no more, no less. Blindly following rules is a bad idea and none of these rules make sense in all possible situtations.

kbw wrote:

If we tweak the example so the two statements are logically equivalent by DeMorgan's rule, we have !(x && y) and (!x || !y).

The first may apeal to sense of readability, but is more inefficent.

In the first case y is only evaluated if x is true and in the second case y is only evaluated if x is false.
Different book, same story.

[jsmith]

The bottom line is that every programmer should strive to write the simplest code possible that solves the problem. There is no such thing as an algorithm/program that is too simple. But there are algorithms and programs that are way more complex than they need to be.
Whatever rules you can apply in support of simplicity, apply them. But as with all other rules, there are no absolutes.