Skip to content

Coding Standards Part 0 Automated Analysis

Kyle Benne edited this page Feb 24, 2015 · 3 revisions
  • [Part 0: Automated Analysis](Coding Standards Part 0 Automated Analysis)
  • [Part 1: Style](Coding Standards Part 1 Style)
  • [Part 2: Performance and Safety](Coding Standards Part 2 Performance and Safety)
  • [Part 3: References and Further Reading](Coding Standards Part 3 References and Further Reading)

C++ Coding Standards Part 0: Automated Code Analysis

Automated analysis is the main advantage to working with a modern statically typed compiled language like C++. Code analysis tools can inform us when we have implemented an operator overload with a non-canonical form, when we should have made a method const, or when the scope of a variable can be reduced.

In short, these tools catch the most commonly agreed best practice mistakes we are making and help educate us to write better code. We will be fully utilizing these tools.

Compilers

All reasonable warning levels should be enabled. Some warning levels, such as GCC's -Weffc++ warning mode can be too noisy and will not be recommended for normal compilation.

GCC / Clang

A good combination of settings is -Wall -Wextra -Wshadow -Wnon-virtual-dtor -pedantic

  • -Wall -Wextra: reasonable and standard
  • -Wshadow: warn the user if a variable declaration shadows another with the same name in the same scope
  • -Wnon-virtual-dtor: warn the user if a class with virtual functions has a non-virtual destructor. This can lead to hard to track down memory errors
  • -pedantic: warn about non-portable code, C++ that uses language extensions.

MSVC

MSVC has fewer warning options, so all warnings should be enabled: /W4. /Wall could be considered, but does not seem to be recommended even by microsoft.

Static Analyzers

Static analyzers look for errors that compilers do not look for, such as potential performance and memory issues.

Cppcheck

Cppcheck is free and opensource. It strives for 0 false positives and does a good job at it. Therefor all warning should be enabled: -enable=all

Clang's Static Analyzer

Clang's analyzer's default options are good for the respective platform. It can be used directly from cmake.

MSVC's Static Analyzer

Can be enabled with the /analyze command line option. For now we will stick with the default options.

Code Coverage Analysis

A coverage analysis tool shall be run when tests are executed to make sure the entire application is being tested. Unfortunately, coverage analysis requires that compiler optimizations be disabled. This can result in significantly longer test execution times.

The most likely candidate for a coverage visualization is the lcov project. A secondary option is coveralls, which is free for open source projects.

Ignoring Warnings

If it is determined by team consensus that the compiler or analyzer is warning on something that is either incorrect or unavoidable, the team will disable the specific error to as localized part of the code as possible.

Unit Tests

There should be a test enabled for every feature or bug fix that is committed. See also "Code Coverage Analysis."

Unit tests should use the Google Test Framework to facilitate running tests and making assertions.

Unit tests should be organized into a series of files, with one test file corresponding to each source code file. The test filename should mirror the source name with the post fix .unit.cc.

Refactoring for Testability

Writing unit tests is challenging because the EnergyPlus source code contains a large number of free functions that share a significant amount of global data. It can be extremely difficult to identify what global data needs to be initialized in order to perform a valid test. Generally the code will require refactoring to make good unit tests. Consider the following guidelines while refactoring.

  • Prefer using object oriented design patterns. Design class constructors to properly initialize member data and strive for well encapsulated classes whose member functions can be tested without using global data.

  • If a class uses external data, prefer to pass this information as method arguments as opposed to accessing global data from within the class implementation.

  • Assuming well encapsulated classes can be extracted, write unit tests for each public and private member function. Private methods can be tested by friending the test fixture class. See for example the sqlite output tests here and here.

  • If it is impractical to refactor code into classes because the changes are too pervasive, prefer to write small, task specific functions that use function arguments as opposed to global data. This may require refactoring large functions into smaller pieces.

Clone this wiki locally