# Simple “nullable” template class. Are there weaknesses in the implementation?

The goal is to have fields in a class which are optional and can be determined if they are set in order to be able to leave them out of serialization.

template<typename T>
class Nullable
{
public:
Nullable() : m_value(), m_isSet(false) {}
Nullable(T value) : m_value(value), m_isSet(true) {}
Nullable(const Nullable& other) : m_value(other.m_value), m_isSet(other.m_isSet) {}

friend void swap(Nullable& a, Nullable& b)
{
using std::swap;
swap(a.m_isSet, b.m_isSet);
swap(a.m_value, b.m_value);
}

Nullable& operator=(Nullable other)
{
swap(*this, other);
return *this;
}

T operator=(T value) { set(value); return m_value; }
bool operator==(T value) { return m_isSet && value == m_value; }
operator T() const { return get(); }

T get() const
{
if (m_isSet) return m_value;
else throw std::logic_error("Value of Nullable is not set.");
}

bool is_set() const { return m_isSet; }

void reset() { m_isSet = false; m_value = T(); }

private:
void set(T value) { m_value = value; m_isSet = true; }

private:
T m_value;
bool m_isSet;
};


• Use const T& and T&& for construct and assignment operators. Also use const T& where you don't modify the values. Use variadic templates with perfect forwarding for versatile in place constructors. – CodeAngry Nov 5 '14 at 14:55

Don't see the point in:

using std::swap;


It adds a line. It is also longer to type than just prefix each swap with std::

As pointed out by @Matt below. There is a good reason to do this.

No point in doing copy-swap idium for a class that is not managing resources.
So really there is no need for the copy constructor or the assignment operator the compiler generated versions will work perfectly well.

If you can compare against a value:

bool operator==(T value) { return m_isSet && value == m_value; }


Why can't you compare against Nullable? The cast operator does not help you here operator T() const { return get(); } as it potentially throws. Yet it is still logical to be able to compare or it should throw if either side has m_isSet as false.

Nullable<int>   x(5);
Nullable<int>   y;

if (y == x){} // works as expected (always false as y is not set).
if (x == y){} // throws an exception.


Also operator== should be const.

Cast and get methods should be returning a reference (big T objects will cost on a copy). You will also want const versions of these functions.

On a reset:

void reset() { m_isSet = false; m_value = T(); }


Why reset the m_value when it can not be accessed anyway when m_isSet is false. Also the cost of setting T here may be expensive for some classes.

As a general comment.
I don't see the point. Each class should have its own serialization operators (>> and <<) and you will need to serialize the whole state for this to work symmetrically. Anything that is not need for serialization is already marked as mutable (ie it is not part of the logical state of the object). Everything else is part of the logical state of the object. Anything that is optional has its own serialize operator and understands when not to serialize its own state.

• Good comments, thank you. As for your general comment, I agree in general, however, this is something I need to fit into an ancient (pre-standard) codebase. Nothing has a serialization operator, they have external classes to serialize, with the pointless opaque getters and setters to get the state of the object. The idea is that these objects are serialized into ini files (yeah, I know, not my idea either) which are edited by humans later, and they want as few clutter as possible in it. I have some int fields which should work like this and I think this is the cleanest possible way to do this. – Tamás Szelei May 16 '13 at 12:59
• Actually, there's a point in using std::swap;--in fact it's a well-known idiom used to enable ADL: stackoverflow.com/questions/3279543/… ; See also: en.wikipedia.org/wiki/Argument-dependent_name_lookup and Section 3.II. in boostpro.com/writing/n1691.html – Matt May 16 '13 at 18:04
• @Matt: yes. I keep forgetting that. – Martin York May 17 '13 at 7:31