Contents
By function I mean:
- global function
- static/instance method
- C++ - General
- Warning level - Level 4 (All builds)
- Make sure your code compiles without any single warning.
- Detect 64-portability issues – Yes (All builds)
- C++ - Code generation
- Smaller type check – Yes (Debug builds only)
- Helps detect constructs in which there is data loss. Works during run time.
- Basic runtime checks – Both (Debug builds only)
- Helps avoid buffer overruns and uninitialized parameters. Works during run time.
- Buffer security check - Yes (Release builds only)
- Helps avoid buffer overruns.
- Runtime library - Multithreaded Debug DLL (Debug), Multithreaded DLL (Release)
- Compilation is faster and code is shared.
- C++ - Language
- Treat
wchar_t as built-in type (All builds)
- ISO C++ conformance (default in VS2005)
- Force conformance in for loop scope - Yes (All builds)
- ISO C++ conformance (default in VS2005)
After switching to warning level 4 (or, what is worse even before) you may encounter many warnings. You can get rid of them by:
- Rewriting some code (recommended)
- Using casting – but be very, I repeat very careful with it.
- Commenting out unreferenced parameters.
- Deleting unreferenced local variables (Why aren’t they used any more?).
- Using
#pragma warning for particular warning (but use it with comment).
- Do not disable warning for the whole file, only the smallest part you have to handle.
- Remember to switch to default warning.
_ASSERTE, ASSERT (MFC/ATL), assert (standard C++)
These macros are removed during preprocessing in release builds along with expressions passed to them so they don’t hurt performance in final build at all.
They take boolean expression as parameters or any expression than can be implicitly converted to boolean.
Always check for correctness of parameters passed to the function (especially pointers). In methods, also check the internal state of the object the function was called on. Trust no one (including you). Check the results of your calculations. Use these macros even for things that you believe are ‘obvious’. Be prepared for invalid parameters, but remember – these macros are removed in the final build so you should also consider the usual checking and don’t pass to assert expressions that have to be evaluated in release builds as well. Remember – assert lines are removed during preprocessing in release builds.
Favor _ASSERTE over other types of assertions when possible, because when assertion fails it will provide a richer message to the tester.
You can also use _ASSERTE in a tricky way. If you just want to signal an unusual state of the application, try using _ASSERTE(!"Some text to display"); instead of _ASSERTE(false); or _ASSERTE(0);.
When assertion (_ASSERTE in this case) fails, you will see a window like this:
Now you can press ‘Retry’ and go to the debugger, check parameters, call stack etc. and find the bug. You can also choose to press ‘Ignore’. In this case further control flow will be exactly as in builds without the assert macro and you can find out what will happen.
But remember – these macros exist only in debug builds. Don’t do like this:
If you want the expression to be evaluated in all builds, use the MFC VERIFY macro. Read more about ASSERT/VERIFY macro in the Visual Studio documentation.
Use exceptions to signal errors: don’t write functions that return special values to signal error, even if this value is false. Some day you will forget to check the returned value!
However, don’t use exceptions as a control-flow mechanism. Do not throw exceptions in destructors, and do not call anything that might throw an exception unless you're prepared to catch it and deal with it.
Improperly used exception mechanism can lead to resource leaks and bugs.
Remember – every C++ function, including [], = operators, constructors, copy constructors can throw an exception unless explicitly declared with throw(). So be prepared!
Check all such values and react somehow - throw exception, return false etc. - on failure. It is tiring? I told you exceptions are better.
If there is nothing you can do when a function fails just use the VERIFY macro. It works exactly the same as ASSERT macro in debug builds. In release builds parameters passed to the macro are still evaluated.
For example, there is nothing you can do if CloseFile fails but at least in debug builds you will know about such an event and you can think a little and figure out why this happened and maybe find some bug(s) in your code…
CString::LoadString(nID) is good for VERIFY macro because it returns FALSE on failure so when it fails you will know that (probably) nID string table entry was not defined.
You can create a Visual Studio macro for fast adding of VERIFY/_ASSERTE.
If documentation says that function returns BOOL, don’t write:
Even if it compiles without any message. Write instead:
This is very important for types like:
- All pointers
HWND
HANDLE
HINSTANCE
UINT_PTR
SIZE_T, size_t
DWORD_PTR
LONG_PTR
LPARAM
WPARAM
It is important because some types’ size changes when compiled for different platforms, like 64 bit. Don’t cast these types to similar ones, e.g. UINT_PTR to UINT, these casts will result in a serious error on 64 bit platforms.
If you have to cast - use C++ style casting. Are you sure you have to?
If you really have to cast, use static_cast. Avoid other casting types, especially reinterpret_cast.
Don’t use constant numbers in your code. Use only values like 0, 1, -1. Don’t use string literals either.
If you really have to use such numbers, use descriptive names and the following solution:
Avoid making these constants global. Limit their scope by declaring them inside the class or inside an anonymous namespace in your CPP file.
Avoid using #define as much as possible. You should replace #define with something that the C++ compiler actually understands so that it can do better error checking: instead of macros use const for constants, enum for related constants, and inline functions.
Change your coloring scheme that numbers are displayed in red so it’s easier to find and avoid such numbers.
Are you sure you really have to use constants at all?
To make sure that your project compiles and works also in Unicode builds, enclose all your string literals in _T() macro. In MBCS builds, this macro does nothing. In Unicode builds, this macro adds a ‘L’ at the beginning of the literal so it is treated by the compiler as a Unicode string.
Use _tcsXXX functions instead of strXXX, but the best way is just use CString methods. Use TCHAR instead of char and const TCHAR* instead of const char*.
Build your project with Unicode switched on sometimes. The best way is just add new Unicode configurations to your project.
Windows NT, 2000, XP, Server 2003, Vista and later Windows operating systems are all Unicode based. MBCS applications run slower on those systems.
Read the documentation about the _T macro in MSDN.
Also use comments when something unusual is done for purpose.
In general, comments should be:
- Brief
- Clear
- Don’t contain information that could be easily found in the code
- Explain why instead of how
- Close to the code they refer to as possible
Remember to update comments while updating code!
If the function only reads such a parameter (of type class or struct) it should be passed by const reference. If the function needs to change it/call non const methods on it, consider using a pointer.
Parameters such as built-in integral types, floating point values etc. should be passed by value unless the function needs to change it.
Don’t copy/paste your code! Consider making a function and call it from different places. If it is small you can mark it as inline, the compiler is smart enough to super optimize it.
Even the smallest function can change so it will be easier and faster to update the project.
Don’t make your functions lengthy. One/two full screens are enough. Prefer simplicity and readability. Split the function if it’s too long/complex.
Don’t make your program logic in CDialog-derived classes. Try separate actual logic (maybe a class or group of classes) in separate cpp/h pair and just use it in your interface class.
Use inline getters/setters instead (but generally such design should be omitted). By doing that, future code changes will be easier + you can check whether a parameter is valid. Do the same with protected variables. Mark getters as const. Avoid extensive use of global/static variables.
Don’t overdo with these functions. Try hiding implementation as much as possible.
Try using Object Orientation/loops instead Avoid also something like this:
- Make the loop/OO do the job!
Don’t design like this:
Do this::
If classes have something in common – just create a base class for them. For example, all CDialog-derived classes in your application should be derived from a common class – CMyApplicationCommonDialogBase. Even if you don’t add new functionality, maybe you will in the future – it will be easy (for example, make all your dialogs have white background).
Every class definition should be structured so that the public section appears first, followed by the protected and finally the private section. When inlining functions, don’t put them inside the class definition unless they fit in only one line of code. If not, put them at the end of the header file.
Always declare a copy constructor and assignment operator in your class to protect it from shallow copy (protect classes from shallow copy). If they are not needed in the code, make them private and don’t define a body.
Consider using explicit keyword on constructors taking only one parameter. This helps avoiding some class of bugs:
Make only non-explicit constructors. If you want, its parameter type can be ‘converted’ to your type.
If there is possibility that a class may be inherited (almost always is), make for that class a virtual destructor.
Beware of the order in constructor initialization lists – it is the same as they were declared in the class not as they appear in the list.
Instead of:
This is important when m_var1 or m_var2 are not built-in types.
There are at least two reasons for doing that:
- If
m_var1 (or m_var2) throws an exception in the constructor – resources are properly cleaned.
- Initialization list is faster in many cases.
Avoid returning from the middle of functions; this creates “jumps” in the code logic and makes the code more difficult to understand and modify. Try to always return either from the beginning or from the end of functions. Return from the beginning if, after checking the parameters, you find that the call was unnecessary; otherwise arrange the function logic so that the function will return from the end.
If you do such returns, maybe the design is wrong – consider splitting the function.
Declare variables that are only assigned a value once and they don’t change as const.
It can be hard to detect bugs.
- In MFC dependant files, put this at the beginning of every CPP file after
#include directives (if it’s not there already).
- Don’t use many inline functions/
__fastcall functions – they really don’t speed up an application so much but causes slower compilation and make code more dependant because they have to be put in a header file (interface is not well separated from implementation). Try using Inline Function Expansion – any suitable compiler option instead (in release builds only). If you think that the whole application will be faster by using such functions you are very wrong. In fact it could be slower… How is this possible? Try figuring it out… Try to address the system performance in a higher level.
- Don’t use custom-but-already-defined functions – if you think you can do something better than standard C++/MFC/ATL library, you are probably wrong.
- Your code should be self-documented – use descriptive variable/class/function names. Choose a name so that the reader will understand variable/class/function purpose without additional comments.
- Embed
enums in your class.
- Declare variables that don’t change as
const. Use the keyword “const” for formal function parameters when the function is not supposed to change them. Use also const after function declarations in a class when the functions are not supposed to change the object they belong to. Use const also on local function variables (for example, when they don’t change during the function/loop).
- Names of identifiers should be chosen so that they enhance readability and writeability.
- Do not
#include a header file for a class that is referred to by pointer or reference only. Instead use a forward class declaration. Avoid code dependencies.
- Avoid deeply nested code.
- Once a method is declared as
virtual in a base class, all derived class methods with the same type signature are virtual, even without the virtual keyword. However, you should carry the “virtual” keyword through derived classes even though it is redundant (it helps readability).
- Avoid multiple inheritance, it can cause problems.
- Use classes instead of structures. The only difference is that structure data members are
public by default
- When deleting an array, use the brackets
[ ]. When deleting something that is not an array, don’t use [ ]. This is a common mistake, and the result is undefined.
- Don’t use base class pointers to access an array of derived objects. Try to avoid arrays of polymorphic objects. Use an array (better:
std::vector) of pointers to objects.
- Most classes should declare (but not necessarily define) the following methods: default constructor, copy constructor, assignment operator, and destructor (possibly
virtual). The reason is to protect against the compiler providing default definitions. You don’t have to define these methods unless these are used in the code, in which case it’s better to have your own methods instead of letting the compiler provide default definitions that are sometimes dangerous bit-wise copy of your objects.
- Implement correctly, the assignment operator and the copy constructor.
- Put always
default in a switch statement. If the code is not supposed to ever execute the default statement, use tricky assert macro and throw an exception. Don’t forget about break!
- Prefer quality over quantity. Think about the future. Favor extensibility.
- Carefully read the documentation!!!
