** 1) How should we format opening/closing brackets ?
>> Always on a new line for me.
I'm with MootPoint's example : as soon as the line with the bracket and the next line are both non-trivial (which happens a LOT in practice), it makes stuff hard to read. Then you'll see people putting an empty line after the line with the brace, which not only kills the advantages but is also even harder to read.
An alternative way of thinking about this is discourage very strongly long lines, put results of temporary computations in variables with explicit names as much as possible, and encourage taking up vertical space instead of horizontal space. That makes the whole "brace on the same line" style more readable, so if we go with braces on the same line it's something we can consider.
** 2) spacing between parens and parameters in method signatures and calls
>> int foo(int a, int b) { : I like that.
It's probably a good idea to basically follow english grammar for code. The one exception I see is the space before parens which I like to leave out, because it's much more portable across programming languages (recent languages are inclined to understand "foo (bar, qux)" as "foo((bar, qux))").
** 3.a) Should we add brackets after a "if" if there's only one operation inside the if ?
>> Certainly not as a rule.
I think it's a very bad idea to state that as a rule. Putting braces helps in a variety of cases, especially while debugging. It makes the code arguably a little harder to read once it's finished writing, even though it's not a big deal when you're used to it. Basically writing them makes stuff marginally easier to write, and marginally harder to read.
But stating it as a rule prevents us from using that for the purpose it was meant : treat boundary cases without too much fuss.
It often happens you want to set default values or reset to something sensible if some value shoots the intended range. In this case, this is not really part of the logic or algorithm, and should be as discreet as possible.
Code: Select all
if (x > 99) x = 99; // Should not shoot 99
...is much more discreet than
Code: Select all
if (x > 99) { // Should not shoot 99
x = 99;
}
...which looks like a big part of the main treatment. No braces makes for lighter code for places where it's simple and not core functionality. It relates to "return early" policies. Lighter code is easier to read.
So yes, in the end enforcing this rule trashes readable pre-conditions and post-conditions. That's why I'm strongly against any rule about this : there are cases where it's much, much more clear without them, and cases where it's much, much more convenient with them.
** 4) Indentation: 2 or 4 spaces ?
> No strong opinion.
I lean toward the 2-end of the spectrum, but I'll be happy with 4. I'd be sad if we had 4 AND braces on a new line AND both braces and inside indented. I don't like this :
Aside from that, 2 or 4 are fine with me.
I think we should maybe state as an additional rule that we use spaces and not tabs to indent.
** 5.a) variable naming: camelcase or lowercase+underscore ?
> CamelCase
No strong opinions on this, I'd be happy with both. I'm against underscores at start and end of identifiers though, which is more discouraged by CamelCase.
** 5.b) variable naming: how to prefix global variables, class attributes, local variables?
> No prefix
I'm against prefixing variables, because it encourages putting a lot of globals and a lot of attributes, and cripples smart completion.
If you think about it :
- Good functions are short, so you see local variables very quickly. Type coloration makes them quite obvious.
- Globals, we of course want to avoid. Most any global should be constant, so it will usually start with a capital letter. Or, you'll have a singleton, so there again capital.
- Attributes, we want to avoid too. It's state, and state makes stuff harder to debug. I know it's harder to avoid them in C++ where there are no closures (amg no closures ;_;), but whenever reasonably possible, state should be avoided.
One important point about prefixing variables is that it goes against smart completion (we all use smart editors don't we ?). When you think of a variable, the first think you think of is its meaning. Prefixing them means you have to go through the whole process of imagining whether it's an attribute or not, by which time you remembered the name in whole. Modern completion works best with names decided by semantic and not by syntax.
** 6) any maximum number of nested blocks depth? (this is of course flexible..it means the coder will be strongly encouraged to refactor when this happens)
> No.
Though I think 3 is a reasonable average, and I'd live with it. It's a nice rule of thumb, but I see little point in stating it as a coding standard. If you have someone dumb enough to write a 5-nested unreadable loop when there are better ways to write it, I do not think the rule will make that person rewrite it in a more sensible way - they'll probably just make huge functions with a lot of calls, try to unroll the loops and generally deal with it in a syntactic manner, which won't help at all.
** 7) any limit to the size of a line ?
> No.
Though, same as before, it may be discouraged to have lines over 80 or 100 characters.
One of the dangers of stating this as a rule is having people just insert line breaks. Which can be good sometimes, but there are times when we're better off writing it another way. I know I'm inclined to do that and have huge tests when I'm *** because I don't want to be moving code around when I'm in a hurry, and that sucks.
Anyway it's hard to state a good general rule about this. Consider the following case :
Code: Select all
if (((x > rect.left && x < rect.left + rect.width) || -1 == rect.width) &&
((y > rect.top && y < rect.top + rect.height) || -1 == rect.height))
{ ... }
Compare to :
Code: Select all
const bool widthDoesntMatter = (-1 == rect.width);
const bool heightDoesntMatter = (-1 == rect.height);
const bool withinXBounds = widthDoesntMatter || (x > rect.left && x < rect.right);
const bool withinYBounds = heightDoesntMatter || (y > rect.top && y < rect.bottom);
if (withinXBounds & withinYBounds)
{ ... }
The first one is easier to write, but with coloration the second one is (arguably ?) easier to read.
Compare :
Code: Select all
if ((WND_FLAG_TRANSIENT | winFlags)
|| (WND_FLAG_NO_BORDER | winFlags)
|| (WND_FLAG_HIDDEN | winFlags)
|| (WND_FLAG_STRANGEMODE | winFlags))
{...}
And :
Code: Select all
const bool transient = WND_FLAG_TRANSIENT | winFlags;
const bool noBorder = WND_FLAG_NO_BORDER | winFlags;
const bool hidden = WND_FLAG_HIDDEN | winFlags;
const bool strangeMode = WND_FLAG_STRANGEMODE | winFlags;
if (transient | noBorder | hidden | strangeMode)
{...}
...I don't see much improvement in writing it the second way, on the contrary. If the first way bothers you, you may as well write a function or a macro to shorten it (local, if possible).
So conclusion : I don't think there is any general rule about this that won't do as much harm as it will do good.
** 8) What order should we use for variable/methods to be declared in .h files? How about order in .cpp files?
> Semantic grouping
I'm strongly against syntactic grouping, it makes for meaningless ****. A striking example is the fact the order of declaration in a class defines initialization order. You'll have conflicting cases.
Also, to give an example of how much suxxance that such a rule yields, I'd rather much :
Code: Select all
class ... {
...
int height();
int width();
int trashable();
}
...rather than :
Code: Select all
class ... {
...
int height();
int trashable();
int width();
}
...this is bad enough like this. Imagine that when you have 40 member functions.
So, same stuff : rule of thumb why not, iron rule no. It's much better to try to make sense. Use alphabetical order when it doesn't matter. Let documentation automatic tools do the automatic ordering.
** 9) Anything else? (we can also discuss that later, we don't need all rules at once)
A number of little things. Suggestions mind you, so that we can think about it, not really rule propositions.
- Maybe we should discourage private in favour of protected. I've run into too much code where the original developer put "private" because it was seen as good practice, but it totally prevents you to do anything good with a subclass. So, protected all the way.
- I like to write constants first. "if (null == foo)" is better than "if (foo == null)". We all had bugs with single equal instead of double, this is a way to make things better.
- "++i" is better than "i++" because it doesn't cause a side effect. I've seen too many perverse and awfully undetectable bugs with i++ that wouldn't have existed if ++i had been the standard way to write it.
Also a little reminder : while it's nice to think about rules, we should consider what happens when we edit code and fix bugs. The prime example is writing comments. An innocent function like this :
Code: Select all
void enstackBlocker(Blocker *foo) { stack->push(blocker); }
...would look silly with any more comment.
But when you realize you have to test whether the stack is created or not, you'll add a null test. And that doesn't need any more comment. And then you'll have two stacks, one for self blockers and one for opponent blockers, based on some attribute, and you'll write a 2-line test. Weeks later, you'll add a line to test color. Weeks later, you'll add a line for destination. None of these changes warrant a comment : all of them are one-liners or close, very explicit, very simple. But the function itself will start looking like a Frankenstein monster.
In this case, there is never any incentive to write a new comment. Actually, adding new lines will make the logic of old lines harder to grasp, but not the logic of what you are writing, so that stating "ADD COMMENTS !" as strongly as you want will not help at all. And when you write new code, you're often not the one who wrote the lines that now need comments, and may not know why and how they where inserted there, which even lowers the incentive to comment.
While it may sound silly, maybe we want to think of a rule that would look like "add comments as soon as the function is over 4 lines", which would prompt someone who adds "obvious" code to comment older code that became not-so-obvious while it's still easy to understand. Also "don't use words that are part of the function name" to alleviate the redundant comments, though I'm not sure that really helps with anything at all.
*** Other
@ MootPoint :
What if I want to put a breakpoint on any of these error conditions? Uh, I can't.
Uh, I'm not convinced we should base our coding standards on the shortcomings of some particular IDE.
I mean, I get your point. It's best to have rules that match our environments. But if someone out there comes that writes code in notepad, and then claims we should have shorter variable names because it's easier to type, I think we'll agree to ask them to switch editors. My point is : we don't want to reduce readability to workaround some weak IDE defect.
@ Jeck :
I find it immensely helpful to have a single sentence synopsis comment following function member definition. Not as a hard rule, but as a strong suggestion? Really, the more comments the better, in my opinion.
While I like comments as much as the next guy, I'm not sure this particular rule helps. If you want a one-line description, the function name is probably the best place in the world to be explicit. I despise the following kind of comment :
Code: Select all
// Get height
int getHeight();
// Get width
int getWidth();
...and I think this rule encourages that. It's not only not helpful, it's actually in the way.
Maybe however we could encourage it for *variables*. It's something I've thought about for a long time, haven't seen it implemented ever, and wonder if it may have good results. Though the variable name is the good place to be explicit about its meaning, maybe its expected bounds, null behavior and all might benefit from being written there.
@ DJardin :
Any restriction on the usage of template and macro ? They can make the one writing them very proud but they can also make the ones using or modifying them very unhappy depending of their complexity ...
Hard to tell. Historically macros have been used for most horrible things. However they sometimes come in handy for cosmetic reasons and make the code look much more beautiful and easier to read than could be done without them.
Maybe we could exclude macros in .h files and require some specific naming scheme for them ? Keep them in the .cpp file for locality, and if you want a specific set of macros for a specific set of source files, group them in an appropriately-named .macro file that you include ? I don't know, just throwing ideas.
Anyway it's clear that macros are often ill-used and it's hard to make them safe. Any r-value macro should be enclosed in (), any non-rvalue macro should be enclosed in "do ... while false" if you really want to avoid any strange side effects with surrounding code... and then you still have your back open to parameters used several times or such. When applicable at all, we should encourage inline static functions instead of macros.
For templates it's harder to state. They don't have the same history of unskillful use, and they are such a powerful, versatile and useful tool that any limitation may get in the way of smarter code.
return point in method. My company forces every method to have a single return point
I don't think that's great. Returning early is a nice way of getting corner cases out of your brain, and often makes for much lighter code.
Code: Select all
if (not_applicable) return; // And don't care any more
I mean, I see your point : it's basically the same as goto being considered harmful, and it has solid grounds - especially when considering habits that also work with newer, better languages. On the other hand, early return sometimes get you gracefuly out of intricate nestings, and I think that especially in C++, that takes it.
@ mnguyen :
First class representation of data.
Very strongly in favor of this.
I tend to think header files should just be header files and not contain any implementation code at all.
The one problem with that is templates. C++ has a number of shortcomings, and one of them is template instanciation needing full source. There are precompiled headers to deal with that but no real unification about them. And templates is much too useful a tool to shun because of such a (relatively to the use of templates) small reason.
We can however decide to ask for templates to go in a different kind of files.