Partners: KDAB Whole Tomato Software CppDepend

23 April 2018

Refactoring with C++17 std::optional

Refactoring with C++17 std::optional

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 such case, but with C++17 there’s probably the most helpful way: std::optional.

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

Intro

Let’s dive into the code quickly.

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

The existing code looks like that:

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 it looks like a code duplication if you call the function is 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 tuple into a separate struct and reduce the tuple to a pair.
  3. Use std::optional to express possible errors.

The Series

This article is part of my series about C++17 Library Utilities. Here’s the list of the other topics that I’ll cover:

  • Refactoring with std::optional (this post)
  • Using std::optional
  • Error handling and std::optional
  • Using std::variant
  • Using std::any
  • In place construction for std::optional, std::variant and std::any
  • Using std::string_view
  • C++17 string searchers & conversion utilities
  • Working with std::filesystem
  • Something more? :)

Resources about C++17 STL:

OK, so let’s refactor something :)

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
  • 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 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> not 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 std::pair<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 out code. We can remove ok and rely on the semantics of the optional.

Just for the 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 its 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 allocations.

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

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

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 be not 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 what was the reason 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 do you think about the proper places where we should use optional.

How would you refactor the first version of the code?
Do you return tuples or try to create structs from them?

See next post in the series: Using std::optional

Here’s some more articles that helped me with this post:

Get my free ebook about C++17!

More than 50 pages about the new Language Standard.

C++17 in detail, by Bartlomiej Filipek

© 2017, Bartlomiej Filipek, Blogger platform
Any opinions expressed herein are in no way representative of those of my employers.
This site contains ads or referral links, which provide me with a commission. Thank you for your understanding.