Using std::optional to invalidate my RAII object in move constructor/asignment

Question

Say I have an RAII class, instances of which should never be copied:

class Session {

public:
    Session(); // Allocates a resource and sets generates a unique Session::id. 
    ~Session(); // Frees the resource

    Session(const Session&) = delete;
    Session& operator = (Session&) = delete;

private:
  std::uint32_t id;
}

Given that I can't allow copies, I'd like to allow moving, so I need to implement the move constructor and move-assignment operator but I'm not sure what I should do with Session::id of the moved instance.

I can either:

1. Set it to some value known to be invalid (maybe change the type to a signed int and use -1 as the invalid value)
2. Use something like std::optional and set it to std::nullopt to invalidate it.

Which (if either) of those options is correct?

Answer 1

Since this can be done without std::optional by just sacrificing one value out of the 2³² possible ids, I'd do like std::string::npos and initialize a constant of the unsigned type your id has with -1.

Example:

class Session {
public:
    // the constant used to signal an invalid id:
    static constexpr std::uint32_t invalid_id = static_cast<std::uint32_t>(-1);

    Session() :
        id(id_counter++)
        /* allocate the resource */
    {}
    Session(const Session&) = delete;
    Session(Session&& other) noexcept :
        id(std::exchange(other.id, invalid_id)) // exchange with the invalid id
        /* move or exchange the resource */
    {}
    Session& operator=(const Session&) = delete;
    Session& operator=(Session&& other) noexcept {
        // check for self-assignment if needed
        std::swap(id, other.id);
        // swap the resource
        return *this;
    }
    ~Session() {
        if(id != invalid_id) { // if the check is even needed
            /* free the resource */
        }
    }

private:
    inline static std::uint32_t id_counter = 0;
    std::uint32_t id;
};

Demo

An option would be to let 0 be the invalid_id if that would feel more natural in your case. You'd just have to initialize invalid_id with 0 and change the id counting to id(++id_counter).

Answer 2

I would only choose the std::optional approach if the class had a single member. So in this case, having a bool member for indicating the validity of an instance of the class seems fine.

Using the bool member you can implement an overloaded operator bool() for the class. This way, expressions like if (session_instance) are possible (instead of writing if (session_instance.id != -1) or if (session_instance.id.has_value()), etc).

An example:

#include <utility>
#include <cstdint>
#include <cstddef>
#include <cstdio>

class Session {

public:
    explicit Session(const std::uint32_t id, const std::size_t size = 10)
    : is_valid { true }, m_id { id }, m_buffer { new char[size] }
    {
        m_buffer[size - 1] = '\0';
    }
    ~Session()
    {
        if ( m_buffer != nullptr )
            delete[] m_buffer;
    }

    Session(const Session&) = delete;
    Session& operator = (const Session&) = delete;

    Session(Session&& rhs) noexcept
    : is_valid { std::exchange(rhs.is_valid, false) },
      m_id { std::exchange(rhs.m_id, 0) },
      m_buffer { std::exchange(rhs.m_buffer, nullptr) }
    {}
    Session& operator = (Session&& rhs) noexcept
    {
        if ( this != &rhs )
        {
            is_valid = std::exchange(rhs.is_valid, false);
            m_id = std::exchange(rhs.m_id, 0);
            if ( m_buffer != nullptr )
                delete[] m_buffer;
            m_buffer = std::exchange(rhs.m_buffer, nullptr);
        }

        return *this;
    }

    operator bool () const noexcept
    {
        return is_valid;
    }

private:
    bool is_valid;
    std::uint32_t m_id;
    char* m_buffer { nullptr };
};

int main( )
{
    Session ses { 1000, 10 };
    Session ses2 { 100, 20 };
    ses2 = std::move( ses );

    if ( ses )
        std::printf( "ses is valid\n");
    else
        std::printf( "ses is NOT valid\n");
}

Note: The operator bool() can also be implemented with return m_id != -1; in case of changing the type of m_id to int. Then no extra bool member will be required. You can simply add a member variable to your class like inline static constexpr int invalid_id { -1 }; and compare m_id with it inside the operator bool().

Answer 3

std::optional is a poor fit because a moved from optional is not disengaged. (example)

Because its default move behavior doesn't match the desired semantics, one would still have to provide custom move operations and the std::optional would not be of much value.

One could write an optional-like type which has the desired move operations. One advantage of this approach is that one could specify the default unused value as a template parameter. One disadvantage of this approach is that classes have more than one member and they are likely all valid or invalid. Therefore, using an optional-like type, after correcting move operations, will lead to moving all class state to another type. That is quite a burden to impose.

On the other hand, one could easily write a flag-like class that provides the indication of whether the instance has been moved from.

class engaged {
public:
    constexpr engaged() noexcept = default;
    constexpr engaged(const engaged&) noexcept = default;
    constexpr engaged& operator=(const engaged&) noexcept = default;
    constexpr engaged(engaged&& other) noexcept : engaged_(std::exchange(other.engaged_, false)) { }
    constexpr engaged& operator=(engaged&& other) noexcept { engaged_ = std::exchange(other.engaged_, false); return *this; }
    constexpr explicit operator bool() const noexcept { return engaged_; }
private:
    bool engaged_ = true;
};

(example)

How would you do it?

There are trade-offs. An RAII type typically has more than a single integer. There are also usually not many such types. It may not matter whether it has been moved from. Generally speaking, I would focus on which RAII types to write and the appropriate interface for each.