5
\$\begingroup\$

Today I implemented a C++11 template class which allows for Nullable types. The reason for this is that std::optional is not yet available, (I use C++11/14) and I wanted to practice a bit, so I decided to make one myself. Also for portability reasons. (The code has to compile on multiple platforms, namely Linux and Windows. GCC/MSVC)

Can you guys take a look at it and point me to some improvements/changes that might be needed?

Here is the code:

Class Definition:

#include <algorithm>

template<typename T>
class Nullable final
{
private:
    union Data
    {
        Data(){};
        ~Data(){};

        Data(const Data&)   = delete;
        Data(Data&&)        = delete;

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

        T m_Data;
    } m_Data;

    bool m_IsUsed = false;
public:
    Nullable() = default;
    ~Nullable();

    Nullable(T object);

    Nullable(const Nullable& object);
    Nullable(Nullable&& object);

    Nullable& operator=(const Nullable& object);
    Nullable& operator=(Nullable&& object);

    Nullable& operator=(const T& object);
    Nullable& operator=(T&& object);



    bool isInitialized();

    void initialize(T&& object);

    void initialize(const T&  object);

    void reset();

    void reset(const T& object);

    void reset(T&& object);

};

Class Implementation: (In same header file)

template<typename T>
void Nullable<T>::initialize(T&& object)
{
    m_IsUsed = true;
    m_Data.m_Data = std::move(object);
}

template<typename T>
void Nullable<T>::initialize(const T& object)
{
    m_IsUsed = true;
    m_Data.m_Data = object;
}

template<typename T>
Nullable<T>::~Nullable()
{
    if(m_IsUsed)
        m_Data.m_Data.~T();
}

template<typename T>
Nullable<T>& Nullable<T>::operator=(const Nullable<T>& rhs)
{
    if(&rhs == this)
        return *this;

    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }

    m_Data.m_Data = rhs.m_Data.m_Data;
    m_IsUsed = true;
    return *this;
}

template<typename T>
Nullable<T>& Nullable<T>::operator=(Nullable<T> && rhs)
{
    if(&rhs == this)
        return *this;

    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }

    m_Data.m_Data = std::move(rhs.m_Data.m_Data);
    m_IsUsed = true;
    rhs.m_IsUsed = false;
    return *this;
}

template<typename T>
Nullable<T>::Nullable(const Nullable<T> & rhs)
{
    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }
    m_Data.m_Data = rhs.m_Data.m_Data;
    m_IsUsed = true;
}

template<typename T>
Nullable<T>::Nullable(Nullable<T> && rhs)
{
    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }
    m_Data.m_Data = std::move(rhs.m_Data.m_Data);
    rhs.m_IsUsed = false;
    m_IsUsed = true;
}

template<typename T>
bool Nullable<T>::isInitialized()
{
    return m_IsUsed;
}

template<typename T>
void Nullable<T>::reset()
{
    m_Data.m_Data.~T();
}

template<typename T>
void Nullable<T>::reset(const T& object)
{
    if(&object == this)
        return;

    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }
    m_Data.m_Data = object;
    m_IsUsed = true;
}

template<typename T>
void Nullable<T>::reset(T&& object)
{
    if(&object == this)
        return;

    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }
    m_Data.m_Data = std::move(object);
    m_IsUsed = true;
}

template<typename T>
Nullable<T>& Nullable<T>::operator=(const T& object)
{
    if(&object == &this->m_Data.m_Data)
        return *this;

    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }

    m_Data.m_Data = object;
    m_IsUsed = true;
    return *this;
}

template<typename T>
Nullable<T>& Nullable<T>::operator=(T&& object)
{
    if(&object == &this->m_Data.m_Data)
        return *this;

    if(isInitialized())
    {
        m_Data.m_Data.~T();
    }

    m_Data.m_Data = std::move(object);
    m_IsUsed = true;
    return *this;
}

template<typename T>
Nullable<T>::Nullable(T object)
{
    m_Data.m_Data = object;
    m_IsUsed = true;
}
\$\endgroup\$
  • \$\begingroup\$ I would use std::aligned_storage instead of the union. \$\endgroup\$ – bobah Jun 17 '17 at 15:37
  • \$\begingroup\$ @bobah Please write all suggestions for improvements as answers, even if they are trivial observations. Comments are for seeking clarification to the question, and may be deleted. \$\endgroup\$ – 200_success Jun 17 '17 at 15:41
  • \$\begingroup\$ Why are you using union Data? Can you point at some reference that shows what you are trying to achieve with this? \$\endgroup\$ – Martin York Jun 17 '17 at 19:09
  • \$\begingroup\$ I want to make a class which can store the type T inside itself. Thus preventing heap allocation. Also, I need it to be default initializable, even if T can't be default initialized normally. AFAIK, according C++ Standard [class.union]/2, I should be okay as long as I define a constructor and destructor for my union. As such, the Union's only purpose is to correctly align memory and being able to not initialize it on construction. But since I am not entirely sure wether this is okay, I decided to ask this very question \$\endgroup\$ – Hindrik Stegenga Jun 17 '17 at 20:25
2
\$\begingroup\$

Naming

I would rename initialize because it's more of a setter. You can usually initialize an object only once (typically in the constructor), but there is no problem with calling initialize twice

Accessing the object

There is no method to access the object. Maybe that's wanted because you are using friendship of some kind that you didn't paste? If that is the case, I highly recommend that you use the Attorney Client idiom if you are not already, so that your friend class/method can only access the m_Data member and not the boolean

Additional features

There are some features you may (or may not) want to add:

  • .get() method
  • T&& constructor
  • bool conversion operator
  • indirection operator
  • structure dereference operator
\$\endgroup\$
  • \$\begingroup\$ Well, the whole point of this class is so i can later initialize a variable. If I would use T as member, then it'd have to initialize it. Either T needs a default constructor or I need to initialize it, which would completely defeat the point of the whole class, wouldn't it? Also, isn't a member in a union not automatically destroyed unless you specifically do so in the constructor? And yes, i forgot the get() method, i already added it :) \$\endgroup\$ – Hindrik Stegenga Jun 17 '17 at 15:46
  • \$\begingroup\$ The whole point is that i don't have to use heap allocation. e.a. unique_ptr. I want to containt the memory used for the object inside the object itself. I am sorry for not clarifying that! \$\endgroup\$ – Hindrik Stegenga Jun 17 '17 at 15:48
  • \$\begingroup\$ (I missread the fact it was an union actually) \$\endgroup\$ – Maliafo Jun 17 '17 at 15:52
  • \$\begingroup\$ I do see a bug now I believe. This is the reason I chose a union: C++ standard [class.union]/2, it states that a union is okay as long as you specifically provide destructors. Which reminds me I have to call the destructor manually too. As long as I call m_Data.m_Data.~T() in my destructor I should be good, right? Edit: Ow wait, i already do that! \$\endgroup\$ – Hindrik Stegenga Jun 17 '17 at 15:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.