Consistent Coding Style
Every developer has their own coding style. I'm not going to tell you to use one style over another. But it is important that you are consistent in your coding style.
Reasoning: If you are consistent in your coding style it makes things like global searches more effective.
Virtual Destructors
Always make your class destructors virtual.
Reasoning: Without virtual destructors you can introduce bugs and memory leaks. I wrote an entire post on this topic alone. There is one exception. If your class has no other virtual functions, and if your class does not derive from a base class, and if you mark your class "final" then you can safely omit the virtual destructor.
Const Function Parameters
Whenever possible, function parameters should be made "const." If a given parameter is not modified by the function, then the parameter should be marked const.
Reasoning: It's great self-documenting code. Just by looking at a function declaration you can tell if a parameter will be modified. It also can help you catch bugs in your code because the compiler may warn you if you accidentally try to violate the const variable.
Const Member Functions
Whenever possible, class member functions should be marked "const." If a given function does not modify a class member variable or call another non-const member function, then the function should be marked const.
Reasoning: It's great self-documenting code. Just by looking at a function declaration you can tell if the class object will be modified. It also can help you catch bugs in your code because the compiler may warn you if you accidentally try to violate the const function.
Static Member Functions
If a class member function does not modify a class member variable nor call a non-static member function, then that function should be marked as "static."
Reasoning: Again, it's great self-documenting code, and it can help you catch bugs in your code because the compiler may warn you if you accidentally try to violate the static function.
No Classes without Member Variables
If you have a class that has no member variables, instead turn that class into either standalone functions or use a namespace to group the functions.
Reasoning: A class with no member variables is not an object at all. By using a class you are wasting a small amount of runtime memory because of things like the "this" pointer.
Properly Size Integers and Floats
When declaring a new integer or (to a lesser degree) a float, consider the range of possible values for that integer and choose accordingly. For example, if the variable will store a person's age then all you need is an 8-bit integer, don't just type "int" and be done with it.
Reasoning: Choosing properly sized variables will help to minimize memory usage during runtime.
Use Signed and Unsigned Integers Appropriately
Most developers seem to use signed integers when an unsigned integer is what they really should have used. For example, a variable to hold a percentage should be an unsigned 8-bit integer since the valid range is 0 - 100.
Reasoning: If you properly use signed and unsigned variables, the compiler will usually generate warnings if you try and improper operation. This helps to prevent bugs in your code.
Use Pre-operators Instead of Post-Operators
Whenever possible use pre-increment (++itr) and pre-decrement (--itr) instead of post-increment (itr++) and post-decrement (itr--).
Reasoning: It turns out that pre-operators are faster than post-operators if the object in question is a class. For simple data types like integers there is no difference. The reason pre is faster than post is because post must make a copy of the original object before performing the operation, but pre does not need to make a copy of the object.
Avoid Large Array Variables
Do not declare large array variables, for example: char buffer[8192]. Instead large arrays like this should be allocated using malloc/new.
Reasoning: All function variables, include arrays, are allocated against the stack not the heap. It's well known the dangers of using a recursive function, you might blow the stack. But large arrays like this could also lead to a stack overflow. So avoid large arrays and malloc/new them instead. It's alright to use small arrays in this way.
Use "bool" and "nullptr" instead of "BOOL" and "NULL"
If your compiler supports them, then use the newer data types bool and nullptr.
Reasoning: By using native types instead of the older #defines the compiler can help you to catch mistakes. For example, if you accidentally type the following the compiler should give you a warning: bool bResult = (n = 5); You meant to type "n == 5" but make a common mistake. Since you used "bool" the compiler should catch this. But if you used "BOOL" the compiler sees this as valid code.
Don't Compare Against TRUE
Do not test for true like this: if(func() == TRUE) or this: if(func() != TRUE) Instead test like this: if(func() != FALSE) and this: if(func() == FALSE)
Reasoning: "TRUE" is defined to be 1, but a function could return something other than 1 to represent true. The only way to guarantee a correct comparision is to only test against FALSE. Of course the best option is to use the newer native type "bool" if it's available.
Use malloc/free Instead of new/delete for Simple Types
If you are allocating memory for native types (ints, floats, chars, enums, or structs containing only these types) then use the older C malloc instead of the newer C++ new. If you are allocating memory for a class object, or a struct containing one or more class objects, then you must use new.
Reasoning: Calling malloc is faster than new. They both allocate memory on the heap, but new then checks for and calls constructors. This means for simple data types malloc is faster.
Do Not Test for NULL After Calling new
If you allocate memory with new, do not test the result to verify the allocation was successful. For example:
CMyClass *p = new CMyClass;
if(p)
{
...
}
Reasoning: This test is a complete waste of time and code. The new operator already tests the allocation. If the allocation fails then new throws an exception. So testing the result yourself is redundant and a waste. Note this is not true of malloc which does not test the result. So if you use malloc or other similar allocator, be sure to verify the result.
Know When to Test for NULL Before Freeing Resources
Some release/free functions test the input before attempting to deallocate them, others do not. For example, CloseHandle on Windows does not test the handle against NULL first. On the other hand, "delete" does test the pointer against NULL.
Reasoning: Not testing first may result in an application crash if the release function does not check. If the release function does check and your code also checks, that is wasted CPU cycles. Learn which functions to check first before calling.
Size Versus Length
When naming things like functions and variables, be aware of the difference between size and length. Size is the number of bytes an item requires whereas length is the count of or the number of items. This is really a problem when it comes to strings (character arrays). With "char" arrays size and length ARE the same, so many developers use "size" and "length" interchangeable. But with newer Unicode and wide strings size and length are different. So name functions and variables accordingly.
Reasoning: Properly named functions and variables reduce the chance of bugs. Since size and length are not always the same, using the wrong one would be a bug.
Do Not Put Conditional Statements on One Line of Code
Do not simplify your code by condensing things down to one line of code. For example:
if(p) delete p;
Instead this should be:
if(p)
delete p;
Reasoning: When you condense code like this it makes it harder to debug. When debugging line by line it is harder to tell if the conditional statement succeeded or failed.
Do Not Use "catch(...)"
Never use "catch(...)" in your code! This form of exception handling catches all exceptions.
Reasoning: Catching all exceptions prevents your code from ever crashing, which is a good thing right? Wrong. In the event of an exception your code continues to execute but it is now running in a "compromised" state. Memory may be corrupted, variables or registers may be wrong. So what ends up happening is your program will crash randomly some place else. These random crashes are impossible to debug postmortem. It is best to remove "catch(...)" and let the original crash occur. Then you can debug and fix the original problem.
Do Not Use Exceptions for Flow Control
Do not use exceptions as a simple way of transferring flow to a different part of the code. For example, if you are deep inside nested loops and it's time to exit, you could throw an exception and catch that exception before leaving that function. Instead you should exit each loop or even use a goto.
Reasoning: When it comes time to debug your code often times it's nice to attach the debugger and tell the debug to break on all exceptions (handled or otherwise). But if you used exceptions for flow control then the debugger will constantly break when no problem has occurred. Exceptions should be reserved for just that, an exceptional condition has occurred that needs special attention. Flow control is not "exceptional."
Only Include Required Files in Your Headers
Do not #include a lot of extra things in your header files. Only include what that file must have.
Reasoning: First, it reduces compile time as the compiler has to open up and parse fewer files. Secondly, if that file gets included somewhere else or in another project, it prevents you from having to pull in a lot of extra unnecessary files.
Forward Declare What You Can in Header Files Instead of Including More Headers
If your header file references a pointer to a class object only, do not #include the header for that class. Instead forward declare the object and put the #include into the CPP file.
Reasoning: The benefits are the same two as the previous. First, it reduces compile time as the compiler has to open up and parse fewer files. Secondly, if that file gets included somewhere else or in another project, it prevents you from having to pull in a lot of extra unnecessary files.
Only Include Required Files in Your CPP File
Do not pull in extra unused headers in your source code files. Make an effort to only include required files, which means removing old headers as code changes over time.
Reasoning: It reduces compile time as the compiler has to open up and parse fewer files.
Avoid Putting Function Bodies Into Header Files
Header files should be for class and function declarations. All the function bodies should be located in CPP files. There are two exceptions; inline functions and templatized functions - both of which many compilers do not allow you to split them across header and source code files.
Reasoning: Again, it decreases the time it takes to compile your code. It also simplifies your headers meaning you do not need to pull in as many files if the header files are used in another project.
Use "#pragma once"
In all of your header files use "#pragma once" and not the older #ifndef guards.
Reasoning: Using pragma once is the only way to guarantee a file is included only once. Problems can occur with guards such as copy and paste bugs (you copied from one file to another and forgot to make the guard unique). Also, if your guard matches a guard from another file (perhaps from an SDK) then a bug might occur.
Beware of Unsafe Macros
As a general rule, do not pass a function into a macro. For example, assume a macro MAX(a,b) Do not do something like this: n = MAX(1000, crypt.CalcCryptoHash());
Reasoning: Do you see the problem with the call to MAX above? Probably not, because the macro hid the details from you. If I expand the macro as the compiler would, the above becomes: n = (1000 < crypt.CalcCryptoHash() ? crypt.CalcCryptoHash() : 1000) ; Now the problem should be obvious, the function CalcCryptoHash() is potentially called twice. At best this is a performance hit, at worst depending on what this function does it could be a bug in your code. So be careful when using macros.
Learn When and Where to Use References
If you are writing a function that takes a pointer (*) as an input, consider using a reference (&) instead. In fact, probably the only time to use pointer instead of a reference is if the caller of the function might pass in NULL/nullptr. But if the parameter can never be NULL/nullptr, then use a reference instead. Also, if the item being passed to a function is an object (class or struct) or even a native type that is larger than the system architecture (e.g. a 64-bit integer on a 32-bit system) then pass by reference as well.
Reasoning: References are more efficient and make for cleaner code compared to regular pointers. When passing things by reference it becomes even more important to use "const" for variables the function will not modify.
