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 4)
7 PagesFirst 2 3 4 5 6  Last
on Aug 01, 2009

Ron Lugge

Not necessarily; there have been occasions when I could have used an int or an unsigned int, and had both work.  I went with unsigned in some cases because I wanted the extra 'range' to my counting; I went with signed in others because it was useful for debugging (or just because there was no point in using unsigned).

If you need the prefix before the variable name to remember that the variable can take values outside the signed int range you have a problem. That should be in the documentation, but not in the variable name. Maybe I read your code and I think you used unsigned because the variable can't be negative, which is a wrong assumption, so using the prefix to make assumptions about how something works is dangerous to say the least.

on Aug 01, 2009

Nights Edge
I would tentatively suggest:
Write unit tests for any core functionality, for components which you intend to share or reuse, and for complex state machines.

Small commits, frequently. Don't keep personal copies of code outside of source control. No-one likes doing mega-merges.

Continous integration is really good to have if you make the effort to have a good set of test (let it be because you use TDD or just because you test your code afterwards). Checking changes in really important places knowing that you have tests to give you a little more or "faith" in your changes is always nice.

on Aug 02, 2009

ckessel
I didn't see much point in getting into the details of the difference between a pointer and an Object Reference and how that's handled... Seemed sufficient to compare it to pass by reference, especially since it's horribly bad practice to assign new objects to the input object reference anyway. Even in Java where it'd be "safe".

Oh I completely agree that it's a horrible practice to reassign input objects, and I can't offhand think of any cases where it would make sense. It's no more safe than doing so in C++ though, unless you delete the object you point to it still wouldn't affect the outside code....not that that makes it a better idea.

Vicente
Continous integration is really good to have if you make the effort to have a good set of test (let it be because you use TDD or just because you test your code afterwards). Checking changes in really important places knowing that you have tests to give you a little more or "faith" in your changes is always nice.

I think unit testing is one of the more underestimated QA elements in programming. It's a pain to write good unit tests, but it's a godsend to have them. I recently had to refactor the types some of our database entities, and without seeing where the unit tests failed due to casting it would have ment clicking around inside our app for hours to test every possible query. Hibernate is my friend, but it is a very stubborn and demanding friend.

on Aug 02, 2009

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.

17) I'm eagerly waiting for the next C++ version to come up with its nice "auto it = pVector->iterator(); it != pVector.end()" instead of having to use typedefs.

18) Does that mean you don't have a compilation server compiling all the check ins in real time? Why would you test compilation AFTER committing?

 

Plus, then you don't have to go and find where the variable is declared in order to know what type it is so that you can use the same type in your for loop.

And you have to rewrite all the variable names if you change the return type of a method

Then came the time to localize GalCiv2 and I had to do a search for quote marks to find all the hardcoded strings.  It was not fun.

Yikes! Poor you.

on Aug 02, 2009

All of that sounds like solid information, but if you don't remind him not to eat your Egg Salad... it's all going to end messy anyway...

on Aug 03, 2009

Clean coding is great!

Help you find things easy, helps your colleagues when trying to make sense of what you wrote. Still it also makes it easier to fire you... something to keep in mind

on Aug 03, 2009

1. Invest in a continuous build tool.  I've used Bamboo and Cruise Control and either are effective - they monitor your source repository and when someone commits, they can fire off whatever you want...typically a compile and then any nunit tests.  They can send emails if someone breaks the build or a unit test.  Bamboo is particularly good about keeping tracking unit test results.

2. I just have to go "uggh" at the Hungarian notation.  A good IDE can tell you a variable's declared type.  Hungarian is painful if you change the underlying data-type.  When you start trying to writing type-neutral code (ie working with templates), it becomes difficult to come up with a good prefix anyway.  I especially dislike putting "C" in front of my class names.  Just capitalizing the first letter sets it apart from functions and variables.  Having a C in front makes it hard on me when I am browsing classes and want to get to the class called WalkAnimator.  Without the C, I can just hit the W key and the list of classes will scroll down to the W's (most gui lists have this sort of functionality).  But when called CWalkAnimator there is no such short-cut.  But I guess this is a personal choice and people have warred over the "right way" for years

3. When you do not care about the result, use the prefix increment/decrement operator instead of the postfix increment/decrement operator.  It is faster, considerably so for the case of classes.  ie write:

++it

instead of:

it++

 

I really like using const.  Since I've been using c# for the past few years instead of c++, I've really missed being able to use const.  I wish they would bring it to the language.  I was surprised to see the comment about std::for_each() being faster than writing a for() loop yourself.  Unless c++ compilers have changed in recent history, the best you can hope for is that the compiler will inline the for loop and also the functor call and you will end up with code that is as fast as a for() loop.  More likely your functor code will be too complex to be inlined and you will end up with slightly slower code (an extra function call each iteration).

 

As an aside, does C++ have anonymous methods yet?  Do they plan on adding them in any fashion?  Being able to write something like :

std::for_each (a.begin(), a.end(), f(x) { x.DoSomething(); });

Is so much easier when writing your code than breaking your stream of thought to go create a new class or function somewhere then come back to what where you wanted to use it.

on Aug 03, 2009

Yes, clean coding is great. However I have not seen it since I started programming. Well not quite true, some simple tasks at the university were clean. I am very happy, that I could move from C++ with MFC to Java. It has compatibilty issues too, but not as much as the mentioned deadly combination.

on Aug 03, 2009

We usually use IClassName for interfaces (pure abstract classes).

Yep, we do that too.  I forgot to put interfaces in there.

Second, don't calculate anything in the main program.  Have all calculations done with functions.  Also, make sure each function is short and don't calculate more than one thing in each.  Splitting up the functions like this helps organize the program as you go along, but the most beneficial part of this is that it allows very easy modification of the program.  Need to add a few features? Just adjust a few functions to call out additional functions that your new features will use.  It also makes sure you understand the interrelationships between the functions, so changing one thing won't adversely affect other things in your program.

 I totally agree.  I touched on this a bit with the one about functions.

Also, The C Programming Language (Ansi C) by Kenigan and Ritchie is one of the best reference books to have around.  It is a very good book for anyone in the industry. (Even if you aren't just a beginner)

I think that was the book I learned C from in college.

Consistent indentation

Oh, yeah, that's a good one.  We use CTRL+F8 a lot.

P.S. I'm really bad at #7. Is there any good tricks to avoid re-inventing the wheel? Its really a waste of my time to try to code something that already exists, and I can't seem to find.

The STL and boost both have a lot of useful algorithms and classes to help with stuff.  CodeProject often has really good stuff too.  That's where we foung PUGXML and a helper class for threads that's come in really useful.  Other than that, Google it. For game related programming resources, Gamasutra has a list on it site and you can also ask at gamedev.net.  I also meant code that exists in our engine already, though, so in that case it's simply a matter of checking with another developer before writing another xml parser or whatever.

Any chance of getting Stardock guidelines for data architecture?

We store data in classes whose data members are private or protected and accessible only by functions (we try to avoid using friends).  Rather than trying to make the class generic, we actually have specfic varaibles for specific members.  We had a programmer on GalCiv2 who introduced us to the concept of property buckets, which map a string ID to another string which holds the value of the field.  While this is useful for temporary storage while reading in for XML, using it for regular classes makes it much harder to debug and adds overhead.  So rather than having a std::map<std::string, std::string> to hold all of the class' data members, we actually declare them normally, like FLOAT m_fStrength, FLOAT m_fSensorRange, etc.  The most generic we get is to have an array of abilities, where the index of the array is defined in an enum.  That way, it's quick and easy to add new abilities but we can still easily view the values in the debugger.  Other than that, I'd have to think about what we do more.

Don't use magic numbers: significant values should not be typed inline, but rather defined somewhere obvious.

Heh, I should have thought of this one.  I complain about it all the time.

For instance the much abused Hungarian notation, works fine as long as you don't change your type.
 

Yes, but why are you changing the type?  Changing type should not be done lightly.  Things like population, which cannot be negative, should be expressed as ULONGs.  Changing it to LONG would be meaningless, and changing it to a FLOAT or DOUBLE would be equally silly.  If you're making a switch from unsigned to signed, you're going to have to make a lot of changes and make certain that everywhere that deals with that variable is still using correct behavior. 

18) Does that mean you don't have a compilation server compiling all the check ins in real time? Why would you test compilation AFTER committing?
  Everyone is expected to check in code that compiles.  But if they forget to check in a file, that will make someone else's code not compile when they update, and if they've gone home for the day already, that's an issue. CVS ignores any files that haven't been added to the project, unlike other version control, which is why that can happen.

I was surprised to see the comment about std::for_each() being faster than writing a for() loop yourself.  Unless c++ compilers have changed in recent history, the best you can hope for is that the compiler will inline the for loop and also the functor call and you will end up with code that is as fast as a for() loop.  More likely your functor code will be too complex to be inlined and you will end up with slightly slower code (an extra function call each iteration).

I had been certain that this was in Effective STL, but maybe I was thinking of the std::find function.  Now I'm going to have to re-read Effective STL.

on Aug 03, 2009

Has there been any talk about Python programming style? I would strongly suggest using PEP8 (http://www.python.org/dev/peps/pep-0008/), as this is something every Python programmer should know. And it makes things consistent if you are using some third party code in mods.

By the way, reading through that PEP is quite informative even if it will not be used for Elemental. Lots of good advices in there!

 

 

on Aug 03, 2009

Has there been any talk about Python programming style? I would strongly suggest using PEP8 (http://www.python.org/dev/peps/pep-0008/), as this is something every Python programmer should know. And it makes things consistent if you are using some third party code in mods.

Unfortunately, PEP8 is in conflict with the style of most major languages (C, C++, Java, C#, etc). Specifically the 79 character line limits and the _ for naming conventions rather than camel case.

Given Stardock is a C++ shop, they'd probably be better suited to maintaining a C++ style in their Python code for overall consistency and avoiding a brain mode-switch when switching code bases.

on Aug 03, 2009

I was surprised to see the comment about std::for_each() being faster than writing a for() loop yourself.  Unless c++ compilers have changed in recent history, the best you can hope for is that the compiler will inline the for loop and also the functor call and you will end up with code that is as fast as a for() loop.  More likely your functor code will be too complex to be inlined and you will end up with slightly slower code (an extra function call each iteration).

A std::foreach will be better than a naive for( int i = 0; i < list.size(); i++ ) doSomethingWith( list[i] ); unless the compiler sees the code is stupid and should rewrites it inself into C-like: for (ptr = list[0]; ptr< endadress; ++ptr) doSomethingWith( ptr );

I personally find the std::foreach ugly and unreadable and prefer the old for, which is fine with iterators except std::vector<whatever>::const_iterator is a pain to write and often requires one or two typedefs.

Yes, but why are you changing the type?

Because the guy before you picked a bad type. A short instead of an int because he thought it was big enough, for instance. Or conversely, an enum when a bool would have been better. Or you template your method and the float argument is now a <T> which may be int, bool or whatever. There are many reasons, including the most common one: We make mistakes and shouldn't shoot ourselves in the foot for having made mistakes. Of course, a game code base has a short life expectancy, but in code which lasts tens of years, prefixing the type is a really bad idea . Examples there: http://mindprod.com/jgloss/unmainnaming.html (item number 31, but the whole site is a must-read).

Everyone is expected to check in code that compiles.  But if they forget to check in a file, that will make someone else's code not compile when they update, and if they've gone home for the day already, that's an issue. CVS ignores any files that haven't been added to the project, unlike other version control, which is why that can happen.

Everyone everywhere is always supposed to check in code that compiles. An integration server does the check automatically and warns people about their mistake without requiring them to manually do a compilation on another machine/clean environment. CVS, SVN, Perforce and other versioning systems I know are all equally unable to guess when a file should be added to a project and when not.

on Aug 03, 2009

I just finished reading through that.  Can I get a couple of Tylenol, please?

on Aug 03, 2009

"Don't use single letter variables."

As someone who has programmed a simple genetic algorithm on my TI-84 calculator, I can attest to how important this is. By the end I needed to write down all the variables and what they were for on a peice of paper, and had ended up using all the letters of the alphabet, some used more than once in the code. Thinking about doing it in more complex code is cringeworthy at the very least.

on Aug 04, 2009

I would strongly suggest using PEP8

We are good at it in my company. Especially at points 3, 6, 11, 14 and 15

for the link.

7 PagesFirst 2 3 4 5 6  Last