Printable View
Hi there,
What coding standards are most important to you? I'm doing a review and would like to get some feedback.
Naming conventions, is m_ used these days?
Class Names - prefixes, case?
Comments - style, on functions?
Do people prefer to use == NULL or == 0?
Any feedback would be great.
Thanks.
The on that I like the most.Quote:
What coding standards are most important to you?
Our company standard is a document of almost 30 pages, discussing them takes some time. What kind of feedback are you looking for ?Quote:
I'm doing a review and would like to get some feedback.
I do like to prefix the class fields with m_ and I do this even in .NET (yes). It helps making a difference between the class members and local variables. And if I ever use global variables I prefix them with g_. I've seen prefixing locals with l_ but I don't like that.
As for class names, I sometimes prefix them with C (especially that the VC++ wizards do that), but other times I just skip the C prefix. Never prefix with C in .NET.
I always prefer comparison with NULL instead of 0, simply because it tells me that is a pointer that is compared, not just some integer value.
I rarely do that, but I don't hate it. I do like to prefix globals with g_, just because globals are problematic enough that you should never be allowed to forget that's what you're working with.Quote:
Originally Posted by diagramatic
Sometimes I'll suffix function parameters with an underscore, especially to differentiate them from class members with the same name. I haven't generalized this to all my functions though.
I prefer classes to be capitalized, global constants and macros to be ALL CAPS, and everything else to be camelCase.Quote:
Class Names - prefixes, case?
Comments should document the usage of a function if it's non-obvious, particularly the meaning of each parameter. Comments for obvious functions are redundant, and should be omitted. If appropriate, postconditions and preconditions should be specified.Quote:
Comments - style, on functions?
For now, NULL. However, C++0x is introducing std::nullptr, and once that is more widely supported I would advocate using it instead. I'm also in favor of omitting the == for simple conditionals, etc if (ptr) rather than if (ptr != NULL).Quote:
Do people prefer to use == NULL or == 0?
It goes without saying that smart pointers should be preferred over raw pointers in most cases, especially now that TR1 has brought so many smart pointer varieties to the standard library. Similarly, STL containers should be preferred over manual memory management in most cases (at least in C++, obviously you can't do that in C). Don't mix new and malloc unnecessarily, use RAII whenever possible for exception safety if nothing else, etc.
Out of interest, what is the benefit of using camelCase?Quote:
Originally Posted by Lindley
A previous employer had a standard of using hungarian notation and since then I've always found it very easy and intuitive to use, eg; I know at a glance that "cType" is a char, "uIndex" is an unsigned integer, "sName" is a string, etc.
I realise that it largely comes down to what you're used to dealing with, but hypothetically if someone with no preference/experience was to ask for a naming convention to use, what would be the advantage of cameCase over hungarian notation?
Nothing. And it is totally not logical either.Quote:
Out of interest, what is the benefit of using camelCase?
Like most of the world does.Quote:
A previous employer had a standard of using hungarian notation and since then I've always found it very easy and intuitive to use, eg; I know at a glance that "cType" is a char, "uIndex" is an unsigned integer, "sName" is a string, etc.
When you have a multiple-word variable or function name, then you have several options for separating the words:Quote:
Originally Posted by petszk
1) Use an underscore and keep everything lowercase, eg, its_a_function,
2) Capitalize each first letter, eg ItsAFunction,
3) Disregard the difference, eg itsafunction,
4) camelCase, eg itsAFunction.
5) Something else?
Now, #1 is very common in C code, so it's best to differentiate C++ by doing something different. #2 I've already said I like to use for class names, so again it's best to do something different. #3 is acceptable, as is #4; perhaps you might like to use one for variable names and the other for function names? Consistency is more important than absolute style.
Hungarian notation has its uses, but it shouldn't be overused. What if you want to change the type of a variable from signed to unsigned? Should you have to go replace every instance of it? Occasionally such a change may introduce a logic error (such as a loop counting down to 0 may not work with an unsigned int counter), but usually not.Quote:
A previous employer had a standard of using hungarian notation and since then I've always found it very easy and intuitive to use, eg; I know at a glance that "cType" is a char, "uIndex" is an unsigned integer, "sName" is a string, etc.
In general, the purpose of a variable is much more important than its type, so that should be emphasized with the variable name.
I have heard the argument of using a different style from #1 in order to differentiate style from the identifiers used in the C++ standard library, and that at least has some merit. I see no value in trying to be different from a common C style for the sake of being different from a common C style in C++.Quote:
Originally Posted by Lindley
Anyway, beyond the issues of style, I suggest reading Sutter and Alexandrescu's C++ Coding Standards.
I find Hungarian notation an annoyance in modern C++ code. First of all, there are simply too many types to provide good abbreviations for. Secondly, due to the reason mentioned by Lindley it inevitably leads to mismatches between the real type of a variable and the type prefix in the variable name. Thirdly, when you write template code you cannot use the same convention, which means your coding standard will become dependent on context.Quote:
Originally Posted by petszk
I find that one too difficult to read.Quote:
Originally Posted by Lindley
I tend not to use underscores, but I do upper case the first letter for each word even if it is just one word.
I don't use Hungarian notations, except I do use a lower case c to prefix a class.
To me it makes a lot of sense.Code:cString String;
cPlayer Player;
cEnemy Enemy;
cObj Obj;
cMp3 Mp3;
I like int* Value;Code:int* Value;
int * Value;
int *Value;
and I do pad a good bit.
Code:int ItsAFunction( int a, int b );
{
if( a == 10 )
{
a = 20;
}
return a;
}
I also tend to align my declarations and separate them by various factorsCode:int Arr[ 1000 ];
Arr[ 1 ] = 0;
Code:cString String;
cPlayer Player;
cEnemy Enemy;
cObj Obj;
cMp3 Mp3;
For coding standards, I generally demand:
1. Comment, comment comment: Sure, your code should be legible without comments, but just because I can read WHAT you're doing doesn't mean I know WHY you're doing it or that it's really what you mean to do. At the start of functions and sections of code, explain what you're about to do. And keep the comments up to date.
2. Things be legible: No wall of code, sufficient white space, long lines broken up in a sane fashion
3. Use simpler steps when possible: Don't cram multiple things onto one line if you can avoid it. Just put it on two lines.
4. Group things up and make functions out of them.
5. If you do something once, you do it the same way afterward: If I go through your code to load a file and it's correct, I don't want to have to go through your next "load file" code line by line also to figure out if that's correct also.
In general for naming practices, I'm pretty easy going. As long as I can read it at a glance and it's descriptive, I'm ok with it. So IMALittleTeaPot and im_a_little_teapot and similar things are just fine with me. However, IMALTP will just make me think you're malted pee or maybe something to do with alt-p... either way, nothing to do with the teapot.
The Hungarian notation was misunderstood from the very beginning. It was supposed to prefix variables based on their kind i.e. what their represent (db column, loop index, name, etc). and not the underlaying type (int, long, array, string, etc.). But it was a misunderstanding because Simonyi (the father of the Hungarian notation) wrote in an academic paper:Quote:
I find Hungarian notation an annoyance in modern C++ code.
But he meant 'kind', not 'type'.Quote:
The basic idea is to name all quantities by their type.
http://msdn.microsoft.com/en-us/libr...8VS.60%29.aspx
Well, that is something I can agree with.
There is no "best" way to do things. Every system has advantages and disadvantages. And unfortunately when you are using libraries that are being maintained by different third parties that can mean that whatever standards you have 'in house' get disrupted by the usage of said libraries.
The important thing is to pick whatever coding standard that you and the rest of the team find easy to use and stick with it throughout the codebase. If there's anything worse than a bad coding standard, it's having a bunch of programmers working on a project and each of them using their own standard. That gets even worse if some of them like to tabify their code and other are hardcore spacers.
* Naming conventions, is m_ used these days?
It is very widely used. and not only in C++. The MFC library uses is, so people on this particular forum will have a tendency to know it well, and likely have adopted the principle in their own writing (when in Rome...).
* Class Names - prefixes, case?
I've seen lots of different ones. Again, pick what you like. The trend again is that people on here will likely follow the way MFC does things because that's what they're using (more of the Rome thing).
* Comments - style, on functions?
Comments... lots of them...
But pointless comments are really really bad also (yes, actually seen this from a "big name" company):
Worse is programmers that like to be terse, then verbose that in the comment.Code:int i = 14; // set i to 14.
When much more appropriate would be something like;Code:bool b = false; // b is used as a variable to hold whether or not we'll enable the control
You don't even need a comment in this case (yes, again, seen lots of the above).Code:bool bEnableControl = false;
0 is a numeric value of zero. NULL is a pointer value. You should use them appropriately. Using 0 when you mean a pointer value is just bad form.Code:Do people prefer to use == NULL or == 0?
Additionally writing 0 where you mean NULL may work on compilers now, but it may very well cause problems in the future when suddenly standards decide that char *p = 0; is no longer valid.
It can also be confusing to someone else. Did you mean to make a char pointer that is pointing to memory location 0? Or did you try to set the char pointed to by the pointer p to 0 ?
The only reason I can see to write 0 instead of NULL is to save typing 3 characters.. But I prefer code clarity over typing a 3 characters a lot more.
Oh, definitely. Personally I set Visual Studio to insert 4 spaces whenever I hit the tab key, and encourage others in my working group to do the same; it's pretty seamless, and avoids a lot of indentation problems.Quote:
Originally Posted by OReubens
Actually, 0 is also a null pointer constant, as is NULL, which is a macro that could well be defined as 0. As such, unless the compiler implements some special checks that are not required by the standard, the use of NULL over 0 provides a false sense of security, although it can provide some clarity, if you are willing to trust it.Quote:
Originally Posted by OReubens
The real solution is nullptr, which should be part of the next version of C++. Consequently, the argument that there is a possibility that "suddenly standards decide that char *p = 0; is no longer valid" is likely to apply for NULL as well. The argument concerning confusion is weak: if the reader could be confused by that syntax, then changing 0 to NULL does not necessarily remove the confusion, e.g., are you trying to set the char pointed to by the pointer p to NULL?
Stroustrup's reason is to avoid the use of a macro, where feasible.Quote:
Originally Posted by OReubens
I like to use tabs and not spaces because it makes it so much easier to remove one tab versus 4 spaces.
Also I want to mention that Code::Blocks does support reformatting of your project source code to a lot of different coding styles. Even making tabs into spaces. One could easily code in a different style and simply run AStyle plugin to convert that style into the standard style. This can help save time and frustration. AStyle is pretty good, there are a few things that can be improved.
Also AStyle can be downloaded and used as a command line, but of course this is harder, but doesn't require Code::Blocks
http://astyle.sourceforge.net/
To be honest, I could probably count on 1 hand the number of times I needed to change a variable's type from signed to unsigned (or vice versa). And if I did need to, yes, I'd change the variables' name as well. It's not like Search & replace is particularly difficult to do.Quote:
Originally Posted by Lindley
Whether you use camelCase, Hungarian notation or something else, changing a variable's type between signed and unsigned may introduce logic errors. I don't see how that is in any way relevant to the camelCase vs Hungarian notation discussion.Quote:
Originally Posted by Lindley
Well this is pretty much how it is. We all have our preferences and I just hate tabs... ;)
I like to format my code for (what I consider) readability and that includes adding spaces here and there. If tabs are used I have to syncronize the settings for every single editor/viewer I come across to be able to use it for code. :(
I know Stroustrup's stance on the matter, it's been quoted on the dev team many times before.Quote:
Originally Posted by laserlight
The thing is. If you're a solo developer, then youc an pretty much do what you like since you're the only one needing to understand your code.
If you're in a habit of working in a team of many developers with new people joining the team on a regular basis, then code readability is much much more of a concern over a lot of other issues people tend to throw around.
* NULL is better at conveying a meaning (a pointer pointing to nowhere), than 0 (a pointer ? a value ?). So it's prefered. So what if it uses a macro ? I see no bad side to it. We already have preprocessing to do a lot of other stuff anyway, so you're not going to save noticably in the area of compiletime, or compiler memory use. And even if, code clarity still is an order of magnitude more important than getting a 0.1s faster compile or having the compiler use 100 bytes less memory.
* If a bunch of macro's can make it easier to understand/write a bit of code, then that's preferred, even if the macro's can be complex on their own. you'll be benefitting from the ease of the macro's a lot more than having to write/maintain the actual macro's themselves.
MFC's messagemaps? Heck yes ! I'd hate to be forced to write all of the underlying bits each and every time.
We have several of those same type of macro's generating tables/functions in our own sources.
You don't get more productivity in a team by saving on macro's, or by saving on typing short identifier names. Stuff like intellisense helps in actually typing the long names. Self explanatory identifiers make life easier if you're maintaining code someone else has written.
The bad side is that the macro conveys a meaning that is not necessarily the truth, i.e., it may be the case that NULL was used by mistake, and actually an integer, not a pointer, really was intended. Contrast this with nullptr, which conveys a meaning that must be, otherwise a compile error will result.Quote:
Originally Posted by OReubens
So, it is not an issue of compile time. It is an issue of whether one can truly trust the clarity that purportedly results.
I agree, if there are no feasible and pragmatic alternatives.Quote:
Originally Posted by OReubens