Coding best practices guide for Torque Game Engine

Code quality can be improved and many errors avoided by the adoption of some simple practices. In this topic we collect a number of best practices, some specific to Torque coding, some not. The goals of these practices are:

• Prevent bugs.
• Produce efficient code.
• Make code more readable and easier for teammates to understand.

Good build, version control habits

• As GarageGames grows to include many people, often working in the same code base, it is important that everyone has good version control etiquette. Every time one person checks in a build that doesn't compile or doesn't run, the entire team is left standing still. It therefore is worth it for every person to take an extra 5, 10, or even 15 minutes before each checkin to make sure the build still compiles and runs and no known bugs were introduced.

Test code before checking in.

• Before checking anything in, thoroughly test it. One should test as broadly as practical. Do not simply test the single feature that you are working on. Often times a new feature introduced will break other features, and a fix for one bug will introduce another. Think about what was changed and be sure to test to make sure other parts of the application were not broken by your change. The best way to verify that your code is doing what you intend it to do is to walk through it in the debugger.

Update before checking in.

• Before checking in code, check out the most recent code. Doing this will make sure your changes compile and work with the most recent version of the source.

View differences between files before checkin.

• View the differences between each file you are checking in and the previous version of that file (do this after first updating). Just like testing code before checking it in will save the team time in the long run, so will viewing differences on files to make sure only intended changes are making it into the repository.

Test in fresh repository before checking in

• After you have checked anything in, check out the build to a fresh copy and test everything again. This will make sure that you didn't forget to add a header file or update the project file.

Good Coding Habits

Use standard Torque coding style

Be familiar with the Torque coding style guide and adhere to it's guidelines. By having all code in the engine follow the same style, it tells the reader that the code was written by professional programmers who are not discovering how to write and organize code as they go. It is particularly important that code produced by GarageGames has a professional look to it since it is intended for public consumption. A consistent professional look also makes it easier to read.

Assert, assert, assert

• Use AssertFatal to test the range requirements for the input and output variables in your methods. This serves to document the function and to catch bugs early.

        F32 testTheNumber(F32 a)
        {
           AssertFatal(a>0.0f && a<1.0f,"testTheNumber: a out of range.");
  
           // do stuff here
  
           AssertFatal(ret>a && ret<2.0f*a,"testTheNumber: return value out of range.");
           return ret;
        }
  

• Whenever dereferencing a pointer, use AssertFatal to catch NULL pointers or handle NULL pointers. Although the program will crash when a NULL pointer is dereferenced, so there is no danger of the bug going undetected, adding an assert will allow a coder who comes along much later to know whether a dereferenced NULL pointer is due to a bad parameter or due to code that doesn't handle all legal inputs.

        Sometimes a pointer should never be NULL:
  
        AssertFatal(walk,"walk should not be NULL here");
        walk = walk->next;
  
        Sometimes a NULL pointer simply needs to be handled:
  
        if (!walk)
           break;
        walk = walk->next;
  
        Sometimes the surrounding logic dictates that a pointer is never NULL:
  
        while (walk)
        {
           // a bunch of stuff
           walk = walk->next;
        }
  
        In the above case, note that the context of code can change over time, so be careful.
  

If-Else Statements

• In an if-else statement, always put the normal case first. Often , the "if" case will be executed more efficiently than the "else" case. Also, all exceptional cases --- cases that occur infrequently --- should be relegated to a separate function. This will make the if-else statement more readable. It also may allow the compiler to better optimize the code.

One Entry, One Exit

• The method of Structured Programming requires that each control structure has one entry and one exit. Thus a loop with several break statements flaunts this principle. We do not take a hard stance on this issue. However, be aware of it, because simple control structures are easier to read and maintain.

Things to Avoid

Deep Nesting

• Avoid deeply nested control structures. After even two levels of nesting, code can become very hard to read and debug. Several strategies can be employed to avoid nesting. For example, break and continue statements can often be used to remove a level of nesting. Large sections of code can be broken off into their own subroutine (avoid doing this if the section of code does not have a single purpose). There may even be some situations that are made more readable by using goto statements (but see below for concerns there). Finally, think hard about your code. It may be possible to reorganize the code in a more understandable form.

        Instead of this:
  
        for (S32 i=0; i<MAX_COUNT; i++)
        {
           if (i&1)
           {
              // bunch of stuff with nasty nesting
           }
        }
  
        Consider this:
  
        for (S32 i=0; i<MAX_COUNT; i++)
        {
           if ( !(i&1))
              continue;
  
           // bunch of stuff with nasty nesting, but slightly less than before
        }
  
        And instead of this:
  
        for (S32 i=0; i<MAX_COUNT; i++)
        {
           if (i&1)
           {
              // bunch of nasty nesting 1
           }
           else
           {
              // bunch of nasty nesting 2
           }
        }
  
        Consider this:
  
        for (S32 i=0; i<MAX_COUNT; i++)
        {
           if (i&1)
           {
              renderLeftHandedObject(i);  // less nesting and even self-documenting
           }
           else
           {
              renderRightHandedObject(i);  // less nesting and even self-documenting
           }
        }
  

Do not assign variables inside an expression

• There used to be a day when whitespace was eschewed and as much code was crammed into as small a space as possible. That was the day of small monitors and slow printers. Some bad coding habits persist from that day. One of those is the assignment of variables inside an expression:

        walk = head;
        while ( (walk=walk->next) != NULL)
        {
           // do stuff
        }
  

  This results in hard to read code, which results in errors.

     Better:
  
        walk = head;
        while ( walk != NULL)
        {
           // do stuff
           walk = walk->next;
        }
  
     Better yet:
  
        for (walk=head; walk; walk=walk->next)
        {
           // do stuff
        }
  

GOTO Statements

• GOTO statements have long been a source of controversy among programmers and computer scientists. For a good discussion of the pros and cons of using GOTO statements see Code Complete chapter 16. There are certain situations under which it is clearly a bad idea to use GOTO statements. Never use a GOTO statement to jump from one code block to another (e.g., from the middle of one loop to the middle of another). Never use a GOTO statement to jump into a deeper level of nesting. On the other hand, realize that break and continue statements are merely fancy GOTO statements. If you want to use a GOTO statement like a "super-break" statement in order to break out of deeply nested code, that might be acceptable. Likewise, if you are using a GOTO statement to insure that some cleanup code is executed, that might also be acceptable. A GOTO statement should only be used in cases where it clearly makes the code more readable.

Template Vectors

• Avoid using template vectors when a simple array will do (template vectors are structures such as Vector<S32>. Vectors are dynamically allocated as the vector grows in size. As the vector grows and is resized, their entire content has to be copied to the new allocation. If you know the general size of the vector, use the resize method to pre-allocate memory for the vector. If you know the maximum size of the vector, it is often preferable to create an array of that size on the stack or with the FrameAllocator instead of using resizeable vectors.
• Avoid creating template Vectors on the stack, and especially avoid doing this in the inner loop of the engine (one would think this goes without saying, but empirical evidence suggests that it doesn't). If you need to have a Vector in an inner loop of the engine, say to hold lists of render images, make the Vector static and clear it before each use.
• In general, consider whether a template vector is being used at a high level or a low level of the engine. If using at a low level, try to find a better way that doesn't result in wasted memory. If using at a high level, then the use of a Vector may be appropriate.

Dead Code

• Do not leave unused files checked into the repository. If a file becomes obsolete, delete it. It can cause a great amount of headache and confusion to someone searching for functions or phrases, or performing global code modifications.
• Do not leave large sections of dead or obsolete code in files. Go ahead and delete it, we have version control that can retrieve it if it becomes necessary to use it in the future.

Always use %g for scanning strings!

Use %g when reading floating point numbers from strings via scanf and brethren. It deals properly with scientific notation (10e30) as well as normal floating point numbers. %f works great until you're working with very large numbers and something turns into a scientific notation number, at which point suddenly it gets turned into zero - imagine the problems!

Depending on the situation, you may want to format your floats for output in various ways. But %g will always read properly.

Use Const when possible

• The const keyword allows you to declare that certain parameters to functions are constant (cannot be changed by the function) and that certain member methods do not change the value of the object. By including the const keyword, the compiler is better able to warn you when you are violating one of your own assumptions (changing a variable you shouldn't be) and is better able to optimize.

        class Foo
        {
           protected:
              U32 mBar;
  
           public:
              U32 getBar() const;
              void setBar( const U32 *newBar );
              const U32 *gemmySomeBarBaby() const;
        };
  
        // Since getBar is delcared as const, it cannot modify any member variables
        U32 Foo::getBar() const
        {
           return mBar;
        }
  
        // Since newBar is const, it cannot be modified by setBar
        void setBar( const U32 *newBar )
        {
           mBar = *newbar;
        }
  
        // Since the return value is const, it cannot be modified by the caller (or
        // assigned to a non-const pointer.
        const U32 *gemmySomebarBaby() const
        {
           return &mBar;
        }
  

• It is good practice to use the const keyword whenever it applies. The reason for this is that if other methods in the library use the const keyword for function access, if you do not restrict your inputs you will be limited by what library functions are available. For example, the following method cannot be used with a non-const string even though nothing it does prevents that.

        char * strnew(char * src)
        {
           char * ret = new char[strlen(src)+1];
           strcpy(ret,src);
           return ret;
        }
  

Wrap static-phase-use variables in functions

In general, all global static variables will be initialized at the start of main(), and all local statics are initialized when their scope is entered. But sometimes you need certain statics to be available during the global static initialization phase. The proper solution in this case (for instance, when you want a bunch of static globals to add themselves to a list) is to have the "master" static initialized in a function's scope, and returned from there, so that it is certain to be available when you need it. For instance:

Vector<U32> &getGlobalVector()
{
   // The real global static we want to use.
   static Vector<U32> realVector;

   // Return it so it can be used!
   return realVector;
}