Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

Refactoring With C++17 std::optional

DZone's Guide to

Refactoring With C++17 std::optional

In this article, a C++ expert takes a look at how to refactor your code to be up-to-date with C++ 17, and how a particular expression can help.

· Web Dev Zone ·
Free Resource

Bugsnag monitors application stability, so you can make data-driven decisions on whether you should be building new features, or fixing bugs. Learn more.

Image title

There are many situations where you need to express that something is "optional" - an object that might contain a value or not. You have several options to implement in such cases, but with C++17 the most helpful way is probably std::optional.

For today I've prepared one refactoring case where you can learn how to apply this new C++17 feature.


The article orginally appeared on my blog: @bfilipek.com:
Refactoring with C++17 std::optional


Intro

Let's dive into the code quickly.

There's a function that takes ObjSelection, for example, which represents current mouse selection. The function scans the selection and finds out the number of animated objects, if there any civil units, and if there are any combat units.

The existing code looks like this:

class ObjSelection
{
public:
    bool IsValid() const { return true; }
    // more code...
};

bool CheckSelectionVer1(const ObjSelection &objList, 
                        bool *pOutAnyCivilUnits, 
                        bool *pOutAnyCombatUnits, 
                        int *pOutNumAnimating);

As you can see above, there are mostly output parameters (in the form of raw pointers), and the function returns true/false to indicate success (for example, the input selection might be invalid).

I'll skip the implementation for now, but here's an example code that calls this function:

ObjSelection sel;

bool anyCivilUnits { false };
bool anyCombatUnits {false};
int numAnimating { 0 };
if (CheckSelectionVer1(sel, &anyCivilUnits, &anyCombatUnits, &numAnimating))
{
    // ...
}

Why is this function not perfect?

There might be several things:

  • Look at the caller's code: we have to create all the variables that will hold the outputs. For sure, if the function is called, it looks like there's code duplication in many places.
  • Output parameters: Core guidelines suggests not to use them.
  • If you have raw pointers you have to check if they are valid.
  • What about extending the function? What if you need to add another output param?

Anything else?

How would you refactor this?

Motivated by Core Guidelines and new C++17 features, I plan to use the following refactoring steps:

  1. Refactor output parameters into a tuple that will be returned.
  2. Refactor a tuple into a separate struct and reduce the tuple to a pair.
  3. Use std::optional to express possible errors.

Let's start!

Tuple

The first step is to convert the output parameters into a tuple and return it from the function.

According to F.21: To return multiple "out" values, prefer returning a tuple or struct:

A return value is self-documenting as an "output-only" value. Note that C++ does have multiple return values, by convention of using a tuple (including pair), possibly with the extra convenience of tie at the call site.

After the change the code might look like this:

std::tuple<bool, bool, bool, int> 
CheckSelectionVer2(const ObjSelection &objList)
{
    if (!objList.IsValid())
        return {false, false, false, 0};

    // local variables:
    int numCivilUnits = 0;
    int numCombat = 0;
    int numAnimating = 0;

    // scan...

    return {true, numCivilUnits > 0, numCombat > 0, numAnimating };
}

A bit better... isn't it?

  • No need to check raw pointers.
  • The code is quite expressive.

What's more, on the caller site, you can use Structured Bindings to wrap the returned tuple:

auto [ok, anyCivil, anyCombat, numAnim] = CheckSelectionVer2(sel);
if (ok)
{
    // ...
}

Unfortunately, I don't see this version as the best one. I think that it's easy to forget the order of outputs from the tuple. There was even an article on that at SimplifyC++: Smelly std::pair and std::tuple.

What's more, the problem of function extensions is still present. So when you'd like to add another output value, you have to extend this tuple and the caller site.

That's why I propose another step: a structure (as it's also suggested by the Core Guidelines).

A Separate Structure

The outputs seem to represent related data. That's why it's probably a good idea to wrap them into a struct called SelectionData.

struct SelectionData
{
    bool anyCivilUnits { false };
    bool anyCombatUnits { false };
    int numAnimating { 0 };
};

And then you can rewrite the function into:

std::pair<bool, SelectionData> CheckSelectionVer3(const ObjSelection &objList)
{
    SelectionData out;

    if (!objList.IsValid())
        return {false, out};

    // scan...

    return {true, out};
}

And the caller site:

if (auto [ok, selData] = CheckSelectionVer3(sel); ok)
{
    // ...
} 

I've used std::pair so we still preserve the success flag, it's not the part of the new struct.

The main advantage that we got here is that the code is the logical structure and extensibility. If you want to add a new parameter then just extend the structure.

But isn't std::pair<bool, MyType> similar to std::optional?

std::optional

From cppreference - std::optional:

The class template std::optional manages an optional contained value, i.e. a value that may or may not be present.
A common use case for optional is the return value of a function that may fail. As opposed to other approaches, such as <T,bool>, optional handles expensive-to-construct objects well and is more readable, as the intent is expressed explicitly.

That seems to be the perfect choice for our code. We can remove ok and rely on the semantics of optional.

Just for reference, std::optional was added in C++17 (see my description), but before C++17 you could leverage boost::optional as they are mostly the same types.

The new version of the code:

std::optional<SelectionData> CheckSelection(const ObjSelection &objList)
{   
    if (!objList.IsValid())
        return { };

    SelectionData out;   

    // scan...

    return {out};
}

And the caller site:

if (auto ret = CheckSelection(sel); ret.has_value())
{
    // access via *ret or even ret->
    // ret->numAnimating
}

What are the advantages of the optional version?

  • Clean and expressive form.
  • Efficient: Implementations of optional are not permitted to use additional storage, such as dynamic memory, to allocate their contained value. The contained value shall be allocated in a region of the optional storage suitably aligned for the type T.
    • Don't worry about extra memory allocation.

The `optional` version looks best to me.

Sorry for a little interruption in the flow :)
I've prepared a little bonus if you're interested in C++17, check it out here:

The Code (std::optional Refactor)

You can play with the code below, compile and experiment:

#include <iostream>
#include <tuple>
#include <optional>

class ObjSelection
{
public:
    bool IsValid() const { return true; }
};

bool CheckSelectionVer1(const ObjSelection &objList, bool *pOutAnyCivilUnits, bool *pOutAnyCombatUnits, int *pOutNumAnimating)
{
    if (!objList.IsValid())
        return false;

    // local variables:
    int numCivilUnits = 0;
    int numCombat = 0;
    int numAnimating = 0;

    // scan...

    // set values:
    if (pOutAnyCivilUnits)
        *pOutAnyCivilUnits = numCivilUnits > 0;

    if (pOutAnyCombatUnits)
        *pOutAnyCombatUnits = numCombat > 0;

    if (pOutNumAnimating)
        *pOutNumAnimating = numAnimating;

    return true;
}

std::tuple<bool, bool, bool, int> CheckSelectionVer2(const ObjSelection &objList)
{
    if (!objList.IsValid())
        return {false, false, false, 0};

    // local variables:
    int numCivilUnits = 0;
    int numCombat = 0;
    int numAnimating = 0;

    // scan...

    return {true, numCivilUnits > 0, numCombat > 0, numAnimating };
}

struct SelectionData
{
    bool anyCivilUnits { false };
    bool anyCombatUnits { false };
    int numAnimating { 0 };
};

std::pair<bool, SelectionData> CheckSelectionVer3(const ObjSelection &objList)
{
    SelectionData out;

    if (!objList.IsValid())
        return {false, out};


    // scan...

    return {true, out};
}

std::optional<SelectionData> CheckSelectionVer4(const ObjSelection &objList)
{   
    if (!objList.IsValid())
        return { };

    SelectionData out;   
    // scan...

    return {out};
}

int main()
{
    ObjSelection sel;

    bool anyCivilUnits = false;
    bool anyCombatUnits = false;
    int numAnimating = 0;
    if (CheckSelectionVer1(sel, &anyCivilUnits, &anyCombatUnits, &numAnimating))
        std::cout << "ok...\n";

    auto [ok, anyCivilVer2, anyCombatVer2, numAnimatingVer2] = CheckSelectionVer2(sel);
    if (ok)
        std::cout << "ok...\n";

    auto retV4 = CheckSelectionVer4(sel);
    if (retV4.has_value())
        std::cout << "ok...\n";
}

Wrap Up

In this post, you've seen how to refactor lots of ugly-looking output parameters to a nicer std::optional version. The optional wrapper clearly expresses that the computed value might not be present. Also, I've shown how to wrap several function parameters into a separate struct. Having one separate type lets you easily extend the code while keeping the logical structure at the same time.

On the other hand, this new implementation omits one important aspect: error handling. Now, there's no way to know why a value wasn't computed. With the previous version, where std::pair was used, we had a chance to return some error code to indicate the reason.

Here's what I've found in Boost:

It is recommended to use optional<T> in situations where there is exactly one, clear (to all parties) reason for having no value of type T, and where the lack of value is as natural as having any regular value of T.

In other words, std::optional version looks ok, only when we accept invalid selection as a "natural" case in the app... that's a good topic for another blog post! I wonder what you think about the proper places to use optional.

How would you refactor the first version of the code?

Do you return tuples or try to create structs from them?

Here are some more articles that helped me with this post:

If you'd like to read more about C++ signupfor my weekly newsletter and grab some C++ bonuses: Bartek's blog newsletter

Monitor application stability with Bugsnag to decide if your engineering team should be building new features on your roadmap or fixing bugs to stabilize your application.Try it free.

Topics:
c++ ,web dev ,refactoring ,code quality

Published at DZone with permission of

Opinions expressed by DZone contributors are their own.

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}