Lead Developer, Stardock Entertainment
Published on July 31, 2009 By CariElf In Elemental Dev Journals

We've got a new coder starting on Monday and in order to help him out, my co-workers and I have been compiling a list of guidelines about our coding practices.  Up until now, I've just sat down with new coders and given them a tour of our codebase and tried to convey our coding practices at that point, but new people always have a lot to remember and having a document to refer to might help.  I thought that I'd share our list with you and see if you have any suggestions to add.

                                          Stardock Dev Guidelines

Read Effective C++ 3rd Edition (More Effective C++ is redundant as it's older than the 3rd edition, which compiled the earlier edition of Effective C++ and More Effective C++) and Effective STL.  You'll recognize some of Scott Meyer's tips in the list below, and it will help you write better code. 

1) Make Get functions (functions that just return a variable) const, unless it's returning a pointer or reference to a class that needs to be modified. Also use const correctness when passing variables.

2) Name variables so that they tell something about the variable.

Don't use single letter variables. Even if it's x y and z for coordinates, use the prefix to indicate what type it is.

Try to avoid generic variable names; instead of ulIndex, use ulShipIndex when iterating through a list of ships.

Class names should have a C in front of them unless they're functor, e.g. CBasicGameObject

Basic types should have a prefix before the variable name:

- b for BOOL,

- n for INT,

- l for LONG,

- ul for ULONG,

- f for FLOAT,

- d for DOUBLE,

- sz for null terminated strings,

- str for STL strings,

- c for CHAR (or TCHAR),

- by for BYTE (unsigned char)

Class member variables should have m_ as a prefix, e.g. BOOL m_bInitialized;

Pointers should have a p in the name, e.g. CMapData * m_pMapData;

References can have an r in the name, e.g. CMapData & rMapData;

Global variables should have g_ as a prefix, e.g. CResourceManager * g_pResourceManager;

3) Use the typedefs for the basic types; if we have to port our code to another platform, it will be easier to add in a header with the #defines. Try to avoid using ints as the size of ints can vary based on the OS, use LONGs instead. Use BOOL instead of bool when possible.

4) Game objects, data classes, and graphic resources should be ref counted. So derive them from CCoreRefCount.

5) STL algorithms make your life easier and can be faster than doing the same work manually. It's faster to call std::for_each on a vector and pass it a functor than to loop through the vector yourself. If you do need to loop through a vector yourself, use iterators instead of accessing the elements with the [] operator, particularly if you're going to delete items.

6) #include "Kumquat3D.h" in every header file.

Try to avoid using #include for other files in your header file. Use forward declarations if possible in the .h, then use #include in the .cpp. It helps with compile times.

7) Check for existing code before you re-invent the wheel, and ask about code you're not familiar with before changing it.

8) Use Skype + IRC. If you don't need an immediate answer to a question, this is less disruptive. It takes at least 15 minutes to get back in the zone once you get out of it. Also, you can seach chat logs for conversations that happened over IM and you can't for verbal conversations.

9) Get a Google e-mail for Google docs, and a Windows Live ID for Mesh.

10) When designing your code, think about if this is code that may be used again. If so, it may belong in Kumquat3D rather than the game project, or write a base class in Kumquat3D and inherit it in the game project so that you can use app specific code.

11) Write detailed change log entries. A text file to be used for the change log is saved with each project.

12) Commit the entire module at once so that there's less chance of code being left out, and so that people who are trying to update while you're comitting your changes won't get partial changes and have to update again when you're done.

13) Don't hard-code strings, use the string file for screens and data files for strings. Also try to avoid hardcoding filenames.

14) Use TCHAR types and their functions, and use _T() for literal strings. Not all of our code uses TCHARs, but we're converting it as we go.

15) Write wrapper functions instead of accessing screen code directly from game code, if it's absolutely necessary that the code be done in the screen code rather than game code.

16) Use this-> when calling member functions.

17) It's often useful to create typedefs for container classes, so that if you change from using a vector to a deque, you don't have to search and replace in your code, and even if you don't, it makes the code more readable.

e.g. typedef std::vector<CBasicGameObject*> BasicGameObjectArray;

18) Try to remember to get someone to update to grab your changes after you've commited them so that you can make sure that you've checked everything in and that everything works on someone else's computer. Or check it out yourself on your testbox and make sure that it compiles.

19) Function names should indicate what the function does. Get means that it's returning a value; if you're calculating the variable, use Calculate or Convert or something of that sort in the title. If you're doing more than one thing in the function, consider whether you should be breaking it up into smaller functions, particularly if you find yourself copying and pasting code in the same function. Breaking the functions into smaller functions may also help you make the task more efficient.


Comments (Page 6)
7 PagesFirst 4 5 6 7 
on Aug 08, 2009

My bad, I had forgotten about static methods. I must say that I think there's no reason to state explicitly this.f() vs. Class.f() because f() name should be obvious enough that it's clear whether it is static or not. I don't think the distinction is worth typing and reading the many additional "this.". I think they clutter the code for little or no added value.

Regarding the Kingdom of Nouns article, I think the author missed the point that verbs have a subject and that English is a SVO language (therefore putting nouns before verbs), but I think that putting this everywhere is a bit like what he blames.

on Aug 08, 2009

LDiCesare
My bad, I had forgotten about static methods. I must say that I think there's no reason to state explicitly this.f() vs. Class.f() because f() name should be obvious enough that it's clear whether it is static or not. I don't think the distinction is worth typing and reading the many additional "this.". I think they clutter the code for little or no added value.

Why f() name should make obvious it's static? What's the value of that? If all your methods have to indicate if they are instance or static methods you are clutering their names

And again, this. also helps differentiate not only static calls, but local variables from instance attributes. The most typical example is in a constructor as Nights Edge said, but in general it helps a lot looking at code and knowing if a method is changing the instance state or not.

on Aug 10, 2009

Vicente

Why f() name should make obvious it's static? What's the value of that? If all your methods have to indicate if they are instance or static methods you are clutering their names

What I mean is that it should be clear reading f name that the method is static. If not, it is badly named, but it doesn't need to have additional stuff on it. Adding "this." in front of all non static methods is what I call cluttering. I'd rather add ClassName in front of all static method calls than this. in front of all non static method calls.


And again, this. also helps differentiate not only static calls, but local variables from instance attributes. The most typical example is in a constructor as Nights Edge said, but in general it helps a lot looking at code and knowing if a method is changing the instance state or not.

Local variables and instance attributes are easy to find with an ide, and if you have to look up farther than one page to find the definition, then the method is porbably too big in the first place. If you really want to make the distinction, I prefer an _ or m_ pre/suffix to using this. because it's shorter and can be skipped when reading, which lets read the code as if it was English. Java and C++ aren't exactly the best languages for having code look like natural language, but the more code looks like language, the easier I find it to understand. I saw some great LISP code which read like English and even a non programmer could understand what it ment. Of course, there were parenthesis everywhere and the non programmer had no clue what they were for, but I find that using symbols that are not pronounced (like "{" instead of "begin") tend to make code easier to read.

on Aug 11, 2009

Vicente


Quoting SoulSpirit2,
reply 70

16) Use this-> when calling member functions.

You can't directly call non-member functions in Java, so it's not a problem

I don't think that's the spirit of that rule. You can't call non-member functions direcly in C# either and that rule is enforced in StyleCop (SA1101 "Prefix Local Calls With This", under Readibility->Member Access). You can read about why to use that rule here:

http://www.thewayithink.co.uk/stylecop/sa1101.htm
I disagree with this rule. If you're doing decent design, most of the access should be to local variables/functions, so this is going to clutter up the code 90% of the time to help with the 10% case. That's completely backwards. Everytime that I see this., I figure that it was written by someone that wanted to invoke intellisense. In that case, the class or method is probably too complicated and needs to be refactored.

 

on Aug 12, 2009

As a (lousy) Python/Javascript developer, this thread makes me feel inadequate.
Kudos every one, this thread is a most interesting read.

on Aug 12, 2009

lol Seboss, ditto that!

on Aug 13, 2009

Well, I don't consider myself a bad developer. But I work in a small team made up of pretty much self taught people that really lacks structure and methods. I wish we could take the time to come up with coding conventions, specs and some methodology.
Unfortunately, our lead developer sees that as pretentious and generally wasteful. So we keep falling in the pitfalls of sloppy, undocumented code. Time to move to another team I guess.

But anyway, I don't know if it was mentionned in this thread yet, but for junior devs out there, I highely recommend reading this book:

The Pragmatic Programmer: From Journeyman to Master

It's not a very long read, it's not very technical, but it's stuffed with excellent advice, funny anecdotes and witty comments on how to go from the garage coder to the real professional.

on Aug 13, 2009

These are all good practices (I might have some different preferences on a couple stylistic issues, but they're preferences, not "better."  )  I do have a couple questions and comments, though.

#1)  Is there a driving reason for continuing to use TCHARs instead of std::wstring or just wchar_t in new code?  Are you waiting until all your legacy char usage has been converted to TCHARs before making the switch?  It just seems a little silly to even be using single-byte chars in modern software if it's possible to make the switch.  Still having chars in the core codebase would be a good reason to hold off, though.

#2) Herb Sutter and Alexandrescu's book C++ Coding Standards (http://www.amazon.com/Coding-Standards-Guidelines-Practices-Depth/dp/0321113586) is extremely good.  Combined with Effective c++, you've covered all the core best practices in these two books.

#3) Kudos for suggesting Skype/IRC before interruptions!  This is so important for productivity!

#4) Addressing a tendency that I've seen in new devs:  Don't be afraid of creating a new class.  If it captures a single concept and has a well-defined responsibility, just do it.  Files aren't scary.  Spaghetti code that results from failing to create files is. 

 

 

on Aug 13, 2009

But anyway, I don't know if it was mentionned in this thread yet, but for junior devs out there, I highely recommend reading this book:

The Pragmatic Programmer: From Journeyman to Master

I picked this up recently and read it. It's showing it's age a bit in that a lot of it's advice has been superceded or absorbed by methodologies like Scrum. If you're a new programmer, it has a lot of value still, but not so much if you've been at it a while.

Disclaimer: my perspective is skewed since software and software process is both my vocation and a hobby. Having read probably 50+ books on software development, my opinion is somewhat "meh" on most books these days since many are rehashing of a mismash of concepts from other books.

on Aug 14, 2009

LDiCesare

What I mean is that it should be clear reading f name that the method is static. If not, it is badly named, but it doesn't need to have additional stuff on it. Adding "this." in front of all non static methods is what I call cluttering. I'd rather add ClassName in front of all static method calls than this. in front of all non static method calls.

A method name should indicate what the method does, not if it's static or instance. I don't really see why the method name should indicate that type of information really, that would be a bad named method in my book, as the decision about if a method is static or instance it's usually related to more things than just the method functionality. But we can check if you want the Java and .NET APIs to see if static methods have names that tell they are static or just that tell what they do.

LDiCesare

Local variables and instance attributes are easy to find with an ide, and if you have to look up farther than one page to find the definition, then the method is porbably too big in the first place. If you really want to make the distinction, I prefer an _ or m_ pre/suffix to using this. because it's shorter and can be skipped when reading, which lets read the code as if it was English.

Using "this" is superior to weird prefixes like m_Var, _var or mVar, as "this" is part of the language while those prefixes aren't. Also, if you can skip m_ I can't see why you can't skip "this", and this.Variable reads perfectly like English while m_Variable doesn't.

LDiCesare

but I find that using symbols that are not pronounced (like "{" instead of "begin") tend to make code easier to read.

I too like more symbols (like C#) than words (like VB), but if you want code that reads like english, nothing beats words like begin, end, and things like that.

By the way, for people interested in design problems, MSDN has a very good resource for that (for .NET, but some ideas are useful everywhere): Design Guidelines for Class Libraries Developers

http://msdn.microsoft.com/en-us/library/czefa0ke(VS.71).aspx

 

on Aug 14, 2009

RalphT

I disagree with this rule. If you're doing decent design, most of the access should be to local variables/functions, so this is going to clutter up the code 90% of the time to help with the 10% case. That's completely backwards. Everytime that I see this., I figure that it was written by someone that wanted to invoke intellisense. In that case, the class or method is probably too complicated and needs to be refactored.

I don't really agree with "decent design = local variables", but if you are doing mostly that, then you are going to have very few uses of this. so it's not going to cause any clutter at all... Also, what's the problem of writting this. to invoke intellisense? There are classes that are long and/or complicated in real life and that are well designed, those things happen, code is not always easy.

on Aug 14, 2009

This conversation has devolved to the point where it is completely over my head.

Either it's been too long since I've had to really use my CSCI vocab (possible), or you're using concepts in ways I haven't been exposed to.  I'm leaning, sadly, to the later.  School:  I go there so that people can teach me what I need to know when they hire me.

on Aug 14, 2009

Don't worry Ron, I'm clueless too.

Just need to keep learnin and soon enough we'll know how to CSCI jargon too.

on Aug 14, 2009

Just need to keep learnin and soon enough we'll know how to CSCI jargon too.

That's only comforting if you're a Computer Sci major in his senior year, about to graduate in two semesters (oh please, let me be right on that!).

on Aug 16, 2009

Vicente

Quoting LDiCesare, reply 78
What I mean is that it should be clear reading f name that the method is static. If not, it is badly named, but it doesn't need to have additional stuff on it. Adding "this." in front of all non static methods is what I call cluttering. I'd rather add ClassName in front of all static method calls than this. in front of all non static method calls.

A method name should indicate what the method does, not if it's static or instance. I don't really see why the method name should indicate that type of information really, that would be a bad named method in my book, as the decision about if a method is static or instance it's usually related to more things than just the method functionality. But we can check if you want the Java and .NET APIs to see if static methods have names that tell they are static or just that tell what they do.

If you know the method and what it does it's pretty clear it's static. Class.forName() doesn't make sense as an instance method for instance.

Regarding Java API, the first non static method I looked up was this one:

public <U> Class<? extends U> asSubclass(Class<U> clazz)

Naming the variable clazz doesn't make sense so I wouldn't pick that (or awt 1.0 which I remember) as good coding practices.

However, the method name is clear enough imo to understand this can't be a static method.

As for .NET API, the first class or interface I find is a helper class, which I'd say stinks. What is an instane of AspNetManagementUtility and why should methods be static or not in the first place? Does it even mean something? I think this interface stinks and so I wouldn't pick .NET api as a good example fo design or naming either.

 

Quoting LDiCesare, reply 78
Local variables and instance attributes are easy to find with an ide, and if you have to look up farther than one page to find the definition, then the method is porbably too big in the first place. If you really want to make the distinction, I prefer an _ or m_ pre/suffix to using this. because it's shorter and can be skipped when reading, which lets read the code as if it was English.


Using "this" is superior to weird prefixes like m_Var, _var or mVar, as "this" is part of the language while those prefixes aren't. Also, if you can skip m_ I can't see why you can't skip "this", and this.Variable reads perfectly like English while m_Variable doesn't.

When I say I prefer _ or m_ because it's shorter than 'this.' I can only agree with you that the even longer 'm_var' is worst. However, you're not answering my point. Go ask a loaf of bread to the baker and sprinkle your sentences with 'this' everywhere you'd want to put them in a piece of code and tell me if that looks like English. Maybe I should say natural language rather than English.



Quoting LDiCesare, reply 78
but I find that using symbols that are not pronounced (like "{" instead of "begin") tend to make code easier to read.

I too like more symbols (like C#) than words (like VB), but if you want code that reads like english, nothing beats words like begin, end, and things like that.

 

No. As I said, some LISP programs read like English and aren't cluttered with begin/end which don't read as English. Then again it's not procedural so it may be harsh to compare LISP to java/C++/C#.

7 PagesFirst 4 5 6 7