AGS Development > Engine Development

AGS Engine Coding Conventions - Proposal Draft 1.0

(1/8) > >>

As we're moving forward with more developers starting to get into the engine code, we need to define a well structured set of conventions which everyone can follow and conform to. This will promote consistent programming from anyone contributing to the source. I will include here a proposal for the AGS Engine Coding Conventions Draft 1.0. Anyone is free to voice their opinion about this proposal, and it will be modified according to popular trend. As a point of reference I have used the Google C++ Style Guide as well as ScummVM's Code Formatting Conventions and Coding Conventions, along with some personal preference.

For the sake of readability and making it easier to update, the AGS Engine Coding Conventions are now in the wiki. This should be considered the "official" draft/proposal (unless otherwise noted).

(Regarding updating, not only can other individuals not edit my posts of course, but also this post was dangerously close to the 30,001 character limit the forums have for a single post, and things were already having to be moved around...)

Crimson Wizard:
Monkey, you are a bit insane, do you know this? :) I still think you could make a separate document, like WyZ did with his Big Plan. I hope you will do that with final CC version.

When will you people ever learn that 99% of everything I do, I do simply because it's fun to do so?

The few games I've released were a blast to make. In fact, in that Monkey Island "parody"(? if it can be called that), I actually had a function called "DoThatSpeechThingThatYouLoveToDoSoMuchB asedOnWhoTheCurrentPlayerIs" or something like that, which was only ever called from within a function simply titled "Do" (of course the source for that is sadly lost in the sands of time, moving, computer hard drive crashes, etc.).

In my OSD game, I programmed hundreds of lines of code to get semi-realistic RPG-style character battle statistics, much of which served absolutely zero purpose in-game.

Sure, I could have put this in the wiki, or made a Google Doc, or etc., etc., but where's the fun in doing that? Creating a huge thread that has to span two separate posts just to that's what I'm all about. :D

Of course, I wouldn't want to see this type of childish behavior exhibited in the AGS engine source code, which is why we need some coding conventions...oh, wait. :P

Crimson Wizard:
Some thoughts on all this.
Please keep in mind, that many points in code conventions are usually stated in sake of consistency rather than better design or optimization.
For these I'll simply mention alternative arguments.

--- Quote ---1.3 Braces
"Curly braces", { and }, should occupy a line by themselves (trailing comments are acceptable also). They should be at one level of indentation lower than the code they contain. This helps as a vertical guide (in addition to the indentation) in finding where a particular block of code starts or ends.

--- End quote ---
I used this style for a very long time, until one day I was asked by our lead dev to put opening brace on the same line as "if" and "for".
He explaned that thus more actual code lines will fit in the screen. At first that may seem a bit ridiculous, but not after you spent months studying hungreds of thousands of lines in 10+ years-old code :)
On other hand, that depends on average function lengths. If we are going to stick to writing smaller functions this may be not that important.

I did not notice if you've mentioned a rule of making each "if" and "for/do/while" contents be enclosed even if they consist of one line. This is sometimes suggested for it would be harder to make a mistake and add new line, forgetting to enclose it in brackets.

--- Quote ---1.5 Naming Conventions
Local variables (inside of functions) can be named using either lower camelCase or c_style_notation, but should remain consistent throughout any single file.

--- End quote ---
Personally I like c_style_notation for function parameters and local vars more for some unexplainable reason :)
Also, if class private data is named using "lower" camelCase, using c_style_notation for locals will help to distinguish them.
There could be an interesting case though if private member's name consists of one word; in that case it would be indistinguishable from local vars. There may be options how to solve this, for example add an extra char as either prefix or postfix.

Regarding function names. There are two main options for how to form a function name, I've seen both being suggested in the web -
a) verb first, e.g. AddTableEntry();
b) noun first, e.g. TableAddEntry().
Personally I suggest this:
- for class members it should start with verb, and should not include owning class name; for example if we have class Table, member function should be AddEntry and not AddTableEntry. (Except if the function is supposed to deal with other object, like CopyEntryToAnotherTable() or something, but I guess this is obvious)
- for global functions it should start with noun, a name of object, or type, or context it refers to; for example: TableAddEntry, or even Table_AddEntry. This will make easier to find function you need, and function declarations will look much better when grouped by context.

Also, what about class names? Should they have "C" or "T" or some other prefix? Or be named plainly?

--- Quote ---No file names (stored in the same location) should differ from each other solely by case.

--- End quote ---
I recently found a "funny" thing about Microsoft Visual Studio 2008 I never thought about before. It puts compiled *.obj files into one folder before linkage, hence making it impossible to have two source (*.cpp) files of the identical name even if they are placed in different directories.
I tried to find a workaround (like make it put them in relative dirs), but failed. People on the web mention proper workaround that is available only for MSVS2010.

--- Quote ---1.10 Pointers and References
For pointers and references, the preferred convention is to have the asterisk or ampersand beside the name

--- End quote ---
You mean "before the name", right?
BTW an example code may help.

--- Quote ---2.14 Integer Types

--- End quote ---
Using integer typedefs is must do in sake of portability!
(Ask JJS about that :))

--- Quote ---3.1 Namespaces and using Directives

--- End quote ---
You MUST mention that example from Google CC that shows how to make an alias for namespaces!
To be honest, I never knew about that. :P

--- Quote ---3.2 Nested Classes
Consider making the classes separate, or using namespaces, unless the nested class is actually part of the structure of the outer class.

--- End quote ---
This should be read as:
- if a type is to be used only internally in the other class's implementation, prefer making it nested type;
- if a type is to be used public relatively often, do not make it nested;
- special case is for types specific for number of classes. Think of STL iterators, each container has a related iterator type; and although iterators are used outside of container pretty much always, they are still made nested types simply because they have identical names (iterator, const_iterator etc).
As an additional note: remember you can always make an alias to the long name by use of typedef.
For example, intead of writing

--- Code: C++ ---CMyClass::CNestedClass::FuthermoreNestedStruct  obj;do this

--- Code: C++ ---typedef CMyClass::CNestedClass::FuthermoreNestedStruct CShorterName; CShorterName obj; 

--- Quote ---3.4 Functions
Use static functions where appropriate, but don't create a new class just to group static functions together (use a namespace instead).

--- End quote ---
I think this needs some elaboration. What do you call "static function"? Do you mean just a function in appropriate namespace, or do you mean non-member function declared as static?

--- Code: C++ ---static void function() {...} The function defined like that will be only callable from inside the object unit it was compiled in. If you try to call such function from other unit, you will get linker error.

--- Quote ---3.5 Local Variables
If a variable is defined inside of a loop then it will be created and destroyed with every iteration of the loop. Particularly when working with class types this may impact performance, so it is acceptable to define the variables outside of the loop using them when it makes sense to do so.

--- End quote ---
Never ever declare an object of container or string type inside a loop! ;)

--- Quote ---5.3 Interfaces
By convention, any interface classes should be named with the suffix "Interface" (e.g., "ClassInterface" or "FooInterface").

--- End quote ---
That is perhaps a case where I would strongly disagree. Adding "Interface" looks a bit lame, and it increases name length for nothing.
What about "I"-prefix? like IFoo?

Aside from all this, I believe that error handling should be discussed at the early stages of further development. This is perhaps not directly a question of code convention, but still worth mentioning.
It isn't always an easy decision. Most basic way is to return integer error code. However, there are situations when having more data along with result code could be useful.
I can name three general options that are well known to me. They all have similar traits though and have some advantages and disadvantages.
1. Return integer error code, but at the same time store additional data in error-handling system. That data will be available for reading by explicitly calling some GetLastError() function.
2. Use plain class object instead of integer as a return value, that may be simply copied and so passed outside nested functions. The object may contain error code, some auxillary data, or even text message. Unless said not to, it may pass this information to the error-handling system on self-destruction; the system will decide if to show any message to the user, write it to the log, or keep silence.
3. Return "error handler", which is declared as a pointer to integer error code, but de facto points to an extendable struct, managed by error-handling system. That handle may be used to get simple result code or cast to struct pointer to get auxilary data.
Or we may use a combo of these.
In any case I believe that
a) error handling must be uniform so far as possible,
b) it is better to have some kind of "error-handling" system (a singleton of some type) that would manage error data and messaging; I think it would be unwise to allow each function to decide what to do with error, since that will not only increase code size and make program vulnerable to non-consistent handling methods, but also counter any attempt to set up a global behavior for error messages.

Alan v.Drake:

--- Quote ---1.1 Spaces vs. Tabs [top]

Tab breaks should not appear in the source files, and should be converted to space characters. Each level of indentation should increase by 2 spaces. You should set the preferences for indentation appropriately in your editor. For other uses of whitespace see 1.3 Whitespace.

--- End quote ---

I do not agree, leading tabs are superior and won't screw up on people with different tab sizes, further alignment can still be done with whitespaces.
But even if you had to forcibly use whitespaces, I'd advise to use at least 3 spaces. 2 is too few.
No seriously 2 is too few, at least 3 or 4!

- Alan


[0] Message Index

[#] Next page

Go to full version