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 7)
on Aug 17, 2009

LDiCesare

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.

So it's not related to the method name at all as you were defending, but to the method functionality.

LDiCesare

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.

The method name? Or the method functionality?

LDiCesare

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.

Hard to argue if you think you would do a better job than the people that designed the .NET and Java APIs just because probably in some place they did something weird (pretty understandable, and they can't fix it in later releases because it would break backwards compatibility). But it's nice that you picked a class in .NET that says the following:

"This API supports the .NET Framework infrastructure and is not intended to be used directly from your code."

In general, with some weird exceptions, I would say the .NET API is pretty well named and designed (following the guidelines I linked earlier when they can, and passing pretty strict StyleCop and FxCop rules).
 

LDiCesare
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.

Code with this in the cases that rule outlines looks perfectly english/natural/well. But I can agree that if you are working with some convention like 80 chars per line or something like that, then this. can be too long for you.

LDiCesare

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#.

You are probably the first person I find that says that something like LISP reads like english, because we know that tons of parenthesis and prefix mathematic operations are so english, mapcar is pretty english too, probably related to driving or GPS

on Aug 18, 2009

The method name? Or the method functionality?

If the method name doesn't reflect the functionality then it's a bad name. So it should be the same. You make a distinction I don't.

 

I picked the first .NET class in the list, alphabetically, I didn't handpick it, it's just the first one I found when you looking up .NET APIs.

Saying that "there are quirks and what are your credentials" is a bit short. Those examples ("clazz" and a mix od static and isntance members) are both in the first classes I looked up. I didn't have to look for such bad examples, and I don't think .NET etc. are necessarily good examples just because they're produced by big firms. And I know that big firms sometimes have to make do with compatibility they'd rather live without. Saying 'look these are good examples except when they aren't and the guys have excuses' doesn't go far.

looks perfectly english/natural/well
not to me, but then English is not my mother tongue.

I said some LISP code, which with macros is readable. It could read like this:

(:total-water-usage ( + (the :hotel :water-usage) (the :bank :water-usage) ) )

They used a 'the' function which looked silly as it didn't seem to explain anything but made reading the code actually meaningful. (It also did some conversions to make sure units were respected.) There were also macros that made function calls read like English, something like (transfer :from-account 1 :to-account 2 :amount 32).

English words were there to provide meaning while parenthesis were a necessary evil. You could explain to a non programmer what the method did quite easily, because they had used some conventions that made it as clear as possible despite the parenthesis clutter. The this you're advocating is just adding the same kind of clutter as those parenthesis in my opinion.

 

on Aug 18, 2009

LDiCesare

If the method name doesn't reflect the functionality then it's a bad name. So it should be the same. You make a distinction I don't.

You guessed the class.forName because that's a pretty easy case in general. With the ASP.NET class you just decided the class was badly designed instead (so I'm pretty sure you wouldn't have guessed which methods were static or not).

But we can go further, in .NET the Object class has two methods called Equals, but one is static and the other not, and the way of knowing it is because of the number or arguments, not because the method name. The same name, the same functionality, but in one case is instance, because it's comparing one object against the instance and the other is static because it's comparing two objects. It's a pretty good example that name has nothing to do with static/instance.

Math libraries are full of those examples too. If I tell you that the class Vector3 has a method called Normalize, is it static or instance or both? You can't answer that, as the three answers make sense and it depends on how the implementer decided to code the library.

LDiCesare

Saying that "there are quirks and what are your credentials" is a bit short. Those examples ("clazz" and a mix od static and isntance members) are both in the first classes I looked up. I didn't have to look for such bad examples, and I don't think .NET etc. are necessarily good examples just because they're produced by big firms. And I know that big firms sometimes have to make do with compatibility they'd rather live without. Saying 'look these are good examples except when they aren't and the guys have excuses' doesn't go far.

Not only they are produced by big firms, they are produced by big firms that employ some of the best talent in the world. So saying that they aren't good examples of design and naming when they are some pretty successfull APIs, with some of the most clever people involved and with a ton of economic resources is well, debatable. And I don't know you, but I know that the people who work on those things are way clever than I am (just reading their blogs shows that they know a lot about what they are doing). You just looked two random classes and ditched the design of both APIs, even if your choosen examples (at least in .NET) were flawed and even when you don't seem to know why they did that, you simply decided it was wrong, instead of thinking there were reasons to do that (even if you don't know or understand them).

LDiCesare

I said some LISP code, which with macros is readable. It could read like this:

(:total-water-usage ( + (the :hotel :water-usage) (the :bank :water-usage) ) )

They used a 'the' function which looked silly as it didn't seem to explain anything but made reading the code actually meaningful. (It also did some conversions to make sure units were respected.) There were also macros that made function calls read like English, something like (transfer :from-account 1 :to-account 2 :amount 32).

English words were there to provide meaning while parenthesis were a necessary evil. You could explain to a non programmer what the method did quite easily, because they had used some conventions that made it as clear as possible despite the parenthesis clutter. The this you're advocating is just adding the same kind of clutter as those parenthesis in my opinion.

At least for me, the following VB:

Function CalculateWaterUsage(ByRef hotelWaterUsage As Integer, ByRef bankWaterUsage As Integer) As Integer
    Return hotelWaterUsage + bankWaterUsage
End Function

Looks pretty english readable... And parenthesis are like semicolons, something you have to put to make code legal. Putting or not this. is made with an intent, you may agree to it or not, but it's totally different.

Edit: I'm not saying the .NET API is perfect, it has rough edges, but that's expected/understandable on such a huge amount of code. But overall, the design is pretty good even if it has some mistakes here and there.

on Aug 18, 2009

#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

A lot of our code still uses char so it would be a huge undertaking to convert everything in one fell swoop.  It took me at least a week just to get the bare minimum of code from our core library converted for ImpulseReactor.

Also, at some point we may want to license out our library, in which case being able to build in either unicode or ANSI would be useful.

on Aug 18, 2009

Did someone already post this?  I don't remember if someone else did and I'm not rereading the thread.

Since a lot of the discussion is about style,  there's some good sections in here about style.

Anyway I like reading about c++ from the mouth of the language's author.

on Aug 18, 2009

 Don't understand a word of this, but I sounds cool

on Aug 18, 2009

Cari-Elf: just curious: given the choice, why would someone want to develop in ANSI-only if they have the option t use Unicode?

on Aug 19, 2009

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.

Visual Assist X helps a lot in that case. I used it when i modded Civilization 4 C++ code, it saves a lot of time when you need to change someone else's code you don't know.

on Aug 19, 2009

Ynglaur
Cari-Elf: just curious: given the choice, why would someone want to develop in ANSI-only if they have the option t use Unicode?

You're confusing 8/16/32 bits and unicode support. They're totally separate topics. You can support Unicode with either 8/16 or 32 bit wide characters.

What are the benefits of 16 bit characters? Not a whole lot unless you're doing a lot of string operations. Just because Stardock is using 8-bit characters doesn't mean that they aren't doing 'unicode' I'll bet that they took a look at the code and schedule and made a concious decision that this wasn't a priority.

I conciously made the decision to support unicode in an older codebase, and it is surprisingly simple to simple put everything in as UTF-8 and convert to/from UTF-16 for the Windows API calls. I also wouldn't be surprised if Python (like XML) requires (or at least supports) UTF-8 for paramter passing.

I'd rather they put their time into gameplay, I jsut don't see the ROI for converting everything (other than the 'cool' factor.)

 

on Aug 20, 2009

RalphT - Thanks for the excellent explanation.  That makes a lot of sense.  I happen to value the "cool" factor of naming, but that's just my Inner Geek.