Modern C++ 101: Refactoring Legacy Code

So here’s a program written in a hurry, possibly in the style of a college CS student in about week six of learning C++ as a taught course. It’s simple, it works, and it can (naturally) be improved upon:

#include <iostream>
#include <cmath>

class Circle {
    static double PI;
    double radius;
public:
    Circle(double r) {
        radius = r;
    }
    double circumference() {
        return 2.0 * PI * radius;
    }
    double area() {
        return PI * pow(radius, 2.0);
    }
};

double Circle::PI = asin(1.0) * 2.0;

int main() {
    Circle a(2.0), b(10), c(1.0f);
    std::cout.precision(15);
    std::cout << "a.circumference(): " << a.circumference() << '\n';
    std::cout << "a.area(): " << a.area() << '\n';
    std::cout << "b.circumference(): " << b.circumference() << '\n';
    std::cout << "b.area(): " << b.area() << '\n';
    std::cout << "c.circumference(): " << c.circumference() << '\n';
    std::cout << "c.area(): " << c.area() << '\n';
}

In the code above, the Circle class defines private two members in lines 5 and 6, the first PI being a static member (sometimes called a class variable in other languages), and the second radius being a regular member. The storage for PI has to be allocated at a single point in the executable, which happens at line 19 where its value is also assigned. To avoid link-time errors, in a real project this line (only) would reside in a separate source file Circle.cpp while the rest of the class definition would reside in Circle.hpp, facts which may cause surprise to novice C++ coders.

The constructor at lines 8-10 initializes radius with its single parameter, and there are two other member functions called area() and circumference() at lines 11-16, both of which take no parameters and return a double. (It is double-precision arithmetic which is used throughout.) The example main program shown above produces the following output:

a.circumference(): 12.5663706143592
a.area(): 12.5663706143592
b.circumference(): 62.8318530717959
b.area(): 314.159265358979
c.circumference(): -6.28318530717959
c.area(): 3.14159265358979

This article aims to explore some of the more modern C++ styles which can be applied to this code, causing as few changes as necessary to the client (user) code in main().

Use of inline static

To get around the need for two source files due to the use of a static class data member, it is possible to use the inline keyword in addition to static:

class Circle {
    inline static double PI = asin(1.0) * 2.0;
// ...

Expose useful constant data

Other parts of the program might want to use a calculated value of PI (as Circle::PI). Declaring it public allows this, but runs the risk of it being changed, affecting all subsequent calls of the member functions which use it. The solution which caters for accidental/malicious access, thread safety etc. is to declare this value constexpr (at best, if possible), or const (at worst):

class Circle {
// ...
public:
    inline static const double PI = asin(1.0) * 2.0;  // check if your system has constexpr <cmath> yet
// ...

Const-correctness

Member functions which do not change any member variables should be declared with const, and this can be applied to the return value, too:

    const double area() const {
        return PI * pow(radius, 2.0);
    }

Don’t call functions unnecessarily

Since pow() is a C Standard Library function, we are probably guaranteed a non-inline function call overhead. However it is not necessary to call it at all, just use:

        return PI * radius * radius;

Part of the reason an exponent operator has never been added to the C++ language is apparently that in real code, many uses of pow() were for low integral indices.

Provide access to member data

What happens if we make a Circle() object and need its radius at a later point in the program? The worst solution would be to make radius public. A better solution is to return it from a member function with a similar name (note: not identical as this is not permitted). From the convention of prefixing data members with d_ or m_ (“data” or “member”), we get:

class Circle {
    double m_radius;
// ...
public:
    const double& radius() const {
        return m_radius;
    }
// ...

Returning a reference is safe in this context, although this version of the code is (currently) equally good without the &.

Use keyword auto wherever possible

Supposing PI and radius were of different types (as we shall allow later), which one should we use for the return value of a function which combines them? Just let the compiler figure it out, and if the surrounding codebase changes, the function will still be correct:

    const auto circumference() const {
        return 2.0 * PI * radius;
    }

Declare functions which won’t throw exceptions as noexcept

Exception handling in C++ adds some overhead to the runtime calling conventions, so use noexcept for more performant (and possibly more space-efficient) code:

    const auto area() const noexcept {
        return PI * radius * radius;
    }

Use inline for small member functions declared but not defined within the class body

All of our member functions are declared within the class body and so are implicitly inline (that is, the function body is reproduced at the call site). When declaring them (only), function definitions using inline are listed in the same header file, those which don’t are found in the implementation file. Be aware that the performance benefit for inlining decreases for longer functions, and the risk of unnecessary code-bloat increases.

Consider making the class generic

Maybe your users are fine using double the whole time, but one day one of them won’t be (embedded systems can sometimes only use float, while supercomputers may offer arbitrary precision floating-point types). We’ll be continuing this theme when we look at how to calculate PI for float and long double, here is just a basic outline:

template<typename T>
class Circle {
    T m_radius;
public:
    inline static const auto PI = // ...
// ...
    const auto& radius() const noexcept {
        return m_radius;
    }
    const auto circumference() const noexcept {
        return T{2} * PI * m_radius;
    }
};

Set the class template type parameter in the constructor

The type(s) of a generic object are needed at the call site, so Circle(2.5) would instantiate a Circle<double>, and Circle(4) would instantiate a Circle<int>. This happens because of the template parameter being specified in the class constructor(s):

template<typename T>
class Circle {
    T m_radius;
// ...
public:
    Circle(T radius) {
        m_radius = radius;
    }
// ...

Use constructor member-initializers

Rather than setting member variables in the body of the constructor, set them beforehand (in the same order they are declared in the class body):

    Circle(T radius) : m_radius{ radius } {}

Note that the constructor function body can be (and often is) empty, but must be present.

Use keyword explicit and move-semantics for constructors

To avoid some implicit conversions use the explicit keyword before the constructor name. To avoid unnecessary copies when large objects (such as arbitrary-precision types) are passed to the constructor use move-semantics:

    explicit Circle(T&& radius) : m_radius{ std::move(radius) } {}

Use constexpr if to return different types from a single function

In our Circle class, we may want to use the library function asinl() to provide a more accurate value with which to calculate PI when instantiating the template type parameter with long double. Conversely, we way want to use asinf() for type float. The following code allows the unthinkable, returning different types (and accuracy of values) from a single function called make_pi():

template<typename T>
class Circle {
    T m_radius;
    static constexpr auto make_pi() {
        if constexpr(std::is_same_v<T, float>) {
            return asinf(1.0f) * 2.0f;
        }
        else if constexpr(std::is_same_v<T, long double>) {
            return asinl(1.0L) * 2.0L;
        }
        else {
            return asin(T{1}) * T{2};
        }
    }
public:
    inline static const auto PI = make_pi();
// ...

The header <type_traits> provides the template is_same_v<>, which when used as above produces a compile-time Boolean stating whether T is the same as float or long double. The three return statements return a different type and value which is permissible because only one of them will be compiled into the object code due to use of if constexpr. (It is assumed that the fall-back to plain asin() uses a multi-precision variant in the case of such a type being used.)

Our final version

Making all(!) of the changes listed above results in the following, assuredly more modern, C++ program:

#include <iostream>
#include <cmath>
#include <type_traits>

template<typename T>
class Circle {
    T m_radius;
    static constexpr auto make_pi() {
        if constexpr(std::is_same_v<T, float>) {
            return asinf(1.0f) * 2.0f;
        }
        else if constexpr(std::is_same_v<T, long double>) {
            return asinl(1.0L) * 2.0L;
        }
        else {
            return asin(T{1}) * T{2};
        }
    }
public:
    inline static const auto PI = make_pi();
    explicit Circle(T&& radius) : m_radius{ std::move(radius) } {}
    const auto& radius() const noexcept {
        return m_radius;
    }
    const auto circumference() const noexcept {
        return T{2} * PI * m_radius;
    }
    const auto area() const noexcept {
        return PI * m_radius * m_radius;
    }
};

int main() {
    Circle a(2.0);
    Circle b(10);
    Circle c(1.0f);
    Circle d(100.0L);
    std::cout.precision(20);
    std::cout << "a.circumference(): " << a.circumference() << '\n';
    std::cout << "a.area(): " << a.area() << '\n';
    std::cout << "b.circumference(): " << b.circumference() << '\n';
    std::cout << "b.area(): " << b.area() << '\n';
    std::cout << "c.circumference(): " << c.circumference() << '\n';
    std::cout << "c.area(): " << c.area() << '\n';
    std::cout << "d.circumference(): " << d.circumference() << '\n';
    std::cout << "d.area(): " << d.area() << '\n';
}

This produces the following output when compiled with latest (trunk) gcc (MSVC doesn’t seem to support better precision from asinl()):

a.circumference(): 12.566370614359172464
a.area(): 12.566370614359172464
b.circumference(): 62.83185307179586232
b.area(): 314.15926535897932581
c.circumference(): 6.2831854820251464844
c.area(): 3.1415927410125732422
d.circumference(): 628.31853071795864768
d.area(): 31415.926535897932384

Just for the record, this code was tested with boost::multiprecision::cpp_dec_float_100 and the values of PI calculated up to 30 figures matched online sources.

In this article we’ve listed some traditional C++ styles, such as const-correctness, and more modern ones, such as use of noexcept and auto. Whether you’re writing your own new code from scratch, or improving an existing codebase that someone else wrote, you should be well-equipped to put many of the Modern C++ jargon terms into practice.

8 thoughts on “Modern C++ 101: Refactoring Legacy Code”

    1. I’m inclined to agree with you, thinking about it more. When area() and circumference() return a “const double” it’s maybe debatable. However, after the class has been made generic, returning “const auto” prevents move semantics, and so must be a “Bad Thing”.

      I’ll update the code.

      Like

    1. Hi,

      Of course, math.h is a C header, so `M_PI` is defined as a macro and as long as you’re using float or double you’ll get maximum possible precision for these types. The latter parts of the article specialize the template class for long double, which would lose some accuracy if you kept on using the macro.

      This article is attempting to promote use of preferring Modern C++’s `constexpr` (over both `const` and `#define`) for constants which can be computed at compile time, in most cases. For the final program, using `constexpr` shouldn’t use any more space than the macro, and should lead to equally performant run-time code (and possibly slightly better than `const`).

      Like

      1. In the namespace std::numbers there are math constants defined like pi. And they are all constexpr. No need to redefine it in your code.

        Like

  1. Which version should be preferred depends on the use case.
    The “modern” version is slower to write, and harder to read, but adds features not in the original specification.
    Does the project you are working on require/benefit from those extra features ?

    Like

Leave a comment