Jon Keating wrote:
On Sunday 24 June 2007 02:16, Erik Johansson wrote:
I would like to get started with merging some code from erijo-dev to
the newapi branch. But first I think we should agree on a common
coding style. I have updated http://trac.licq.org/wiki/CodingStyle to
reflect a discussion I had with Anders on #licq some time ago. I'm now
posting it here so that Jon (and others) can give their input as well.
I've given some opinions in the past and we talked on IRC awhile ago as well.
The current version is acceptable for me. The only thing left is the
Commenting section. I think the following points should be added to it:
* Use Doxygen style comments for all function/class definitions. [1]
It shouldn't matter which way to write them but my vote is on /**,
mostly because that's the way I've already started commenting new
classes in the qt4-gui.
* When a bug is fixed, add comments to the changed code and reference the
bug number.
Not sure I agree here. For smaller bugs, like grammatical errors in
printouts or memory leaks where the fix is just to correct or add
something that should have been there all along, is there really
anything to gain by having a comment in the code that says "There used
to be a memory leak here, #12345." ?
For larger bugs I would say that only if the ticket contains a longer
description or discussion that explains why a certain
implementation/workaround has been used will there be a point in
referring to it.
But either way I think it is better to have the comment explain the code
directly, if possible, rather than just referring to a bug.
What I don't want to see is something like this:
int x = <huge expression that makes no sense> + 1; // #12964
* Don't add useless comments [2]
Pretty obvious but I agree that it should be stated in the coding style
as someone might need to be reminded.
Anything else that you can think of?
I'd also like to add the following:
* Comments should normally reside on their own lines, example:
// This is a good comment
int x = y;
int z = w; // This is a bad comment as it will make lines very long
If declaring a lot of variables, or some other code with a lot o similar
lines where each line needs just a few words in the comment, it might
look better to have the comments on the same line if each comment is
fairy short.
Example:
switch (i)
{
case 1: return 2; // Short comment
case 4: return 10; // Short comment
case 8: return 5: // Short comment
... And so on ...
* Indentation in switch blocks should be described. As the 'case' lines
are only labels and not statements, some will not add an indentation
step for them but I think it improves readability.
Example:
// Indent like this:
switch (i)
{
case 1:
x = p + u;
break;
case 2:
...
// Not like this:
switch (i)
{
case 1:
x = p + u;
break;
case 2:
...
* Global variables should be mentioned. Add the usual text that they're
evil and then describe a naming convention (if we have any) for the rare
cases when they are used anyway.
Currently there are at least a few in the GUI and they are prefixed with
a 'g' (i.e. gMainWindow).
* The usage of NULL vs. using 0 should be specified. I remember
discussing this in #licq and I think it should be specified which to
use. (I still say that NULL is a C constant and that 0 is that thing to
use in C++ but I can accept 0 if that's what everyone else wants to use.)
* Specify whether the following is good or bad:
if (x) return;
With my syntax highlighting the return keyword is very visible but I
still think it should be two lines, like this:
if (x)
return;
* Should we mention anything on how to compare booleans?
// Is this good?
if (isActive() == true)
if (isFinished() == false)
// or should they be written like this?
if (isCompleted())
if (!isRunning())
Even though the exclamation mark may be harder to miss, I'm in favor of
using it rather than comparing with false.
* Add a statement that code should give no compiler warnings. And it
might be helpful to describe how to handle unused parameters
int SomeClass::someFunction(int \* unusedParam1 *\, void* /*
unusedParam2 */)
That's all I can think of at the moment. I'll probably add more later.
/Anders
