Table of Contents

In this blog post, I’d like to show you how I could quickly improve my old project with Modern C++. Thanks to using the newest compilers and free code analysis checkers you can cover and modernise a lot of code.

Intro  

If you have a terrible code like:

float* pfloats = new float[10]; // no delete [] later! :)
int x = pfloats[0];

You can quickly come up with the issues here and how to fix them. Even the basic compiler will also tell you about a narrowing conversion error from float to int.

But how about some larger code samples? And your commercial projects at work?

In Visual Studio 2019 (but also in VS 2015 and later) there’s code analysis functionality you can enable and have some meaningful insights about the code.

For a start see this code:

#include <iostream>

class SuspiciousType {
public:
    SuspiciousType() { }
    ~SuspiciousType() { std::cout << "destructor!\n"; }

    int compute(int z) { return x + y + z; }

    int x;
    int y;
};

int main() {
    SuspiciousType st;
    float* pfloats = new float[10]{ 100.5f };
    int z = pfloats[0];
}

In Visual Studio 2019 16.4, we can go to projects options and select rules that fit your needs. You can enable all of them, or focus on some smaller “profile” of the code.

When I enabled the code analysis I got the following issues:

For the SuspiciousType class:

cpptests.cpp(5): warning C26495: Variable 'SuspiciousType::x' is uninitialized. Always initialize a member variable (type.6).
cpptests.cpp(5): warning C26455: Default constructor may not throw. Declare it 'noexcept' (f.6).
cpptests.cpp(6): warning C26432: If you define or delete any default operation in the type 'class SuspiciousType', define or delete them all (c.21).
cpptests.cpp(6): warning C26447: The function is declared 'noexcept' but calls function 'operator<<<std::char_traits<char> >()' which may throw exceptions (f.6).
cpptests.cpp(8): warning C26440: Function 'SuspiciousType::compute' can be declared 'noexcept' (f.6).

And later here are the warnings for the code in the main() function:

cpptests.cpp(16): warning C26462: The value pointed to by 'pfloats' is assigned only once, mark it as a pointer to const (con.4).
cpptests.cpp(17): warning C26496: The variable 'z' is assigned only once, mark it as const (con.4).
cpptests.cpp(17): warning C26481: Don't use pointer arithmetic. Use span instead (bounds.1).
cpptests.cpp(16): warning C26409: Avoid calling new and delete explicitly, use std::make_unique<T> instead (r.11).
cpptests.cpp(16): warning C26400: Do not assign the result of an allocation or a function call with an owner<T> return value to a raw pointer, use owner<T> instead (i.11).

That’s quite nice! Visual Studio reported all of the essential issues.

What’s more, in many cases, when a rule comes from a C++ core guideline, you can see there’s a rule number mentioned at the end of the comment. You can open core guidelines and just look up the rule.

Here’s the link to all of the guidelines: C++ Core Guidelines @Github

Another thing is that the lines that are found to be suspicious are now shown with a green squiggle line, and when you move the mouse over those lines, you can get messages as tooltips:

One note: if you cannot use the latest Visual Studio, you can also have a look at Clang Power Tools that allows you to check your code with clang-tidy integration. Have a look at this website: https://clangpowertools.com/

Ok, but I wrote that lousy code intentionally, … can we use it on something “real”?

Checking a larger project  

At the beginning of December 2019, I dig out my old project from the studies. It’s an application that visualises sorting algorithms. I wrote it in 2005/2006 and used old C++, Win32Api and OpenGL.

Here’s the app preview:

Above you can see a cool animation of quick sort algorithm. The algorithm works on an array of input values and performs a single step around 30 times per second. The input data is then taken and drawn as a diagram with some reflection underneath. The green element is the currently accessed value, and the light-blue section represents the part of the array that the algorithm is working on.

While the app looks nice, It has some awful ideas in the code… so please don’t blame me much :)

Looking at code that I wrote so long time ago is a positive experience. Since I like the app, I decided to convert it to a VS 2019 project and then start refactoring it. For example, initially, the application didn’t have the quick sort algorithm, so I implemented it and that way I also “recalled” how to work in that system.

The application uses C++03… or something like that :), so having such a “toy” is an excellent thing as you can experiment and modernise the code in many ways. It’s not massive, but it’s also not that super small (it’s around 5k LOC).

Reported issues  

Ok, back to the code analysis… can something go wrong in a project written that long time ago? Maybe I was super smart at that time, and I wrote amazing code that’s still valid in 2020?

Yeah… right :)

I enabled all of the code analysis warnings… and I got 956 of them!

Let’s have a look at some more substantial warnings.

Use const  

The compiler can see that you don’t change the variable and suggests using const. For example, for code like:

case cmYawPitchRoll: {
    float r = cos(m_fPitch);
    float x = r*sin(m_fYaw);
    float y = sin(m_fPitch);
    float z = -r*cos(m_fYaw);
    m_vTarget = VECTOR3D(x, y, z);
    m_vUp = VECTOR3D(sin(m_fRoll), cos(m_fRoll), 0.0f);
    break;
}

The warning:

Warning    C26496    The variable 'r' is assigned only once, mark it as const (con.4).

It can even suggest using constexpr for functions like:

// ang * M_PI / 180.0f
inline float DegToRad(float a) { return a*0.01745329252f; };  
// rads * 180.0f / M_PI
inline float RadToDeg(float a) { return a*57.29577951f; };

The warning:

Warning    C26497    The function 'DegToRad' could be marked constexpr if compile-time evaluation is desired (f.4).

Variables that are not initialised  

That’s a common error in my code, unfortunately! It’s easy to initialise all variables when you create a class, but then, when you add new member variables, I forgot to initialise them:

For CGLFont:

CGLFont(): m_FontMode(fmNone), m_iList(0), m_iTexture(0) { }

But I forgot about m_fSize.

The message:

Warning    C26495    Variable 'CGLFont::m_fSize' is uninitialized. Always initialise a member variable (type.6).

Reduce the use of pointers  

Back in 2005, I didn’t know much about smart pointers, so I used new and delete all the time.

Now in Modern C++ we should really avoid such code and Visual Studio can easily find places to be updated:

g_Algorithms[ABUBBLE_SORT] = new CBubbleSortAlgorithm();
g_Algorithms[ASHAKER_SORT] = new CShakerSortAlgorithm();

And the message:

Warning    C26409    Avoid calling new and delete explicitly, use std::make_unique<T> instead (r.11).

The compiler can detect issues with null pointer checks and for example reported:

Warning    C26429    Symbol 'avSystem' is never tested for nullness, it can be marked as not_null (f.23).

Render(CAVSystem *avSystem) {
    ColorType ct;
    avSystem->BeginDrawing(1.0, (int)m_vArray.size());
    ...

So I should decide if the pointer can be null in this situation or not.

nullptr modernisation  

That’s an easy thing, but in all the places where I used NULL, I can now replace that with nullptr from C++11.

There are even clang-tidy features to do that automatically.

Use noexcept  

While my code used exceptions to some extent, I didn’t write consistent code in that regards. With C++11 we got noexcept, and now, for every function, we should decide what to do.

In dozens of places the compiler reported:

Warning    C26440    Function 'CBeat::SetTempoBPS' can be declared 'noexcept' (f.6).

For code like:

void SetTempoBPS(double fTempo) { m_fTempo = fTempo; }
void SetTempoBPM(double fTempo) { m_fTempo = fTempo/60.0; }
double GetTempoBPS() { return m_fTempo; }
double GetTempoBPM() { return m_fTempo*60.0; }

Not to mention, getters should be const…

More noexcept  

Contrary to setting noexcept for every function, sometimes we’d have to remove this specifier or consider updating the function.

For example, I got:

Warning    C26447    The function is declared 'noexcept' but calls function 'Destroy()' which may throw exceptions (f.6).

For:

CGLApp::~CGLApp() {
    Destroy();
}

Modernising code with override  

Back in 2005 there was no support for override so when I had an interface that defined three pure virtual functions:

// in the interface
virtual void Init(CViData *viData) = 0;
virtual void Step() = 0;
virtual void Stop() = 0;

I had no way to express that in a derived class, so I just used:

// in derived:
void Init(CViData *viData);
void Step();
void Stop();

With C++11 we can, of course, change it and mark as

// in derived:
void Init(CViData *viData) override;
void Step() override;
void Stop() override;

Rule of Zero  

For whatever reason I defined a lot of empty destructors for my classes and the compiler reports now:

Warning    C26432    If you define or delete any default operation in the type 'class CCamera', define or delete them all (c.21).

That’s a classic Rule of Zero, and I should reconsider my classes, should they only expose constructors or maybe I need some extra resource handling inside?

Summary  

Returning to an ancient project is fun, especially if you liked the project’s idea. It’s amazing how tools changed over time. With the help of a modern compiler and code analysis, I could detect a lot of issues and fix them. It’s like having a colleague who does a basic code overview. Additionally, through such checks and refactoring, you can learn a lot about Modern C++.

Going back to the title of this text: You can rely on your “force”, knowledge and experience and see through the code and improve it. But you can also try modern C++ tools (Visual Studio is just one of them, but there are others on other platforms) and modernise code even easier.

The project repo github/fenbf/ViAlg-Update

Back To You

  • Do you use code analysis tools?
  • Do you sometimes refactor old code? What tools do you use?

References  

If you’d like to know more about Visual Studio Code Analysis:

The core guideline checkers are installed by default in Visual Studio 2017 and Visual Studio 2019, and are available as a NuGet package for Visual Studio 2015.