HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Triple-type template container

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
typetripletemplatecontainer

Problem

I made a triple-type container, that is, a single type that can store a value, under one of the three types used. It's somewhat similar to Boost::Variant.

The header file (multitype.hpp):

```
#ifndef MULTITYPE_HPP
#define MULTITYPE_HPP

#include
#include
#include

typedef void* vp_t; // Void pointer (can contain every type)

template struct is_same_type { const static bool value = false; };
template struct is_same_type { const static bool value = true; };

template
class TripleType
{
protected:
enum ct_t { _tA, _tB, _tC, _init_ }; // Type flags
struct abc_t
{
A __a; // A container
B __b; // B container
C __c; // C container
ct_t _e; // Which type is the current one?
};
private:
abc_t val;
public:
TripleType()
{
(this->val)._e = _init_;
}
template TripleType(T x)
{
vp_t yv; yv = &x;
bool is_A = is_same_type::value;
bool is_B = is_same_type::value;
bool is_C = is_same_type::value;
if (is_A) { A t = static_cast(yv); val.__a = t; val._e = _tA; }
else if (is_B) { B t = static_cast(yv); val.__b = t; val._e = _tB; }
else if (is_C) { C t = static_cast(yv); val.__c = t; val._e = _tC; }
}
template TripleType& operator=(T rhs)
{
vp_t yv = &rhs;
bool is_A = is_same_type::value;
bool is_B = is_same_type::value;
bool is_C = is_same_type::value;
if (is_A) { A t = static_cast(yv); (this->val).__a = t; (this->val)._e = _tA; }
else if (is_B) { B t = static_cast(yv); (this->val).__b = t; (this->val)._e = _tB; }
else if (is_C) { C t = static_cast(yv); (this->val).__c = t; (this->val)._e = _tC; }
return *this;
}
template T get()
{
vp_t yv

Solution

-
Don't make it possible for your container to contain nothing. It can be wrapped in a std::optional for that. Drop the _init_ type and parameterless constructor.

-
Use val consistently. The parentheses around this->val are unnecessary anyway, but since you're already using val in some places, just use it everywhere.

-
Don't allow construction or assignment with values of bad types. That fails silently right now, which is not a good way to fail. Luckily, you can do even better and keep the static typing with overloading:

TripleType(A x) { val._e = _tA; val.__a = x; }
TripleType(B x) { val._e = _tB; val.__b = x; }
TripleType(C x) { val._e = _tC; val.__c = x; }

TripleType& operator=(A x) { val._e = _tA; val.__a = x; return *this; }
TripleType& operator=(B x) { val._e = _tB; val.__b = x; return *this; }
TripleType& operator=(C x) { val._e = _tC; val.__c = x; return *this; }


-
Mark methods that don't modify their object – get() and current_type() – as const.

-
Don't allow fetching values of the wrong type. I'm not a C++ expert by any means, so there might be a better way of restricting T to A, B, or C while keeping the same .get calling convention (I sure hope so), but:

private:
    A get_(A const*) const {
        if (val._e != _tA) {
            throw bad_multitype_access(typeid(A), current_type());
        }

        return val.__a;
    }

    B get_(B const*) const {
        if (val._e != _tB) {
            throw bad_multitype_access(typeid(B), current_type());
        }

        return val.__b;
    }

    C get_(C const*) const {
        if (val._e != _tC) {
            throw bad_multitype_access(typeid(C), current_type());
        }

        return val.__c;
    }

public:
    template  T get() const {
        return get_(static_cast(nullptr));
    }


where bad_multitype_access would be something along the lines of:

class bad_multitype_access : std::logic_error {
public:
    bad_multitype_access(std::type_info const& expected, std::type_info const& actual) :
        std::logic_error(std::string("Attempted to get ") + expected.name() + " from multitype containing " + actual.name()) {}
};


That's all verbose and a bit hacky, but prevents getting types outside of the expected set statically and values of the wrong type dynamically. Again, I would love to hear a better way of accomplishing the same thing from someone who actually knows the language. You can also drop the vp_t typedef at this point, which was never really a good idea, as well as is_same_type.

-
Use a switch for current_type().

std::type_info const& current_type() const {
    switch (val._e) {
    case _tA: return typeid(A);
    case _tB: return typeid(B);
    case _tC: return typeid(C);
    }
}


-
Stop it with the underscores already! =)

private:
    enum contained_type {
        tA,
        tB,
        tC,
    };

    struct {
        A a;
        B b;
        C c;
        contained_type type;
    } val;


-
It feels like creating a TripleType shouldn't create an instance of each of its possible types, and that writing a new value of a different type should destroy a previously existing one. unique_ptrs could work here.

private:
    ⋮
    struct {
        std::unique_ptr a;
        std::unique_ptr b;
        std::unique_ptr c;
        contained_type type;
    } val;
    ⋮
        return *val.a;
    ⋮
        return *val.b;
    ⋮
        return *val.c;

public:
    TripleType(A x) { val.type = tA; val.a = std::make_unique(x); }
    TripleType(B x) { val.type = tB; val.b = std::make_unique(x); }
    TripleType(C x) { val.type = tC; val.c = std::make_unique(x); }

    TripleType& operator=(A x) {
        val.type = tA;
        val.a = std::make_unique(x);
        val.b.reset(nullptr);
        val.c.reset(nullptr);
        return *this;
    }

    TripleType& operator=(B x) {
        val.type = tB;
        val.a.reset(nullptr);
        val.b = std::make_unique(x);
        val.c.reset(nullptr);
        return *this;
    }

    TripleType& operator=(C x) {
        val.type = tC;
        val.a.reset(nullptr);
        val.b.reset(nullptr);
        val.c = std::make_unique(x);
        return *this;
    }


-
Avoid the double-copy-construct by accepting T const& instead of T in the constructors and operator=s.

-
Since TripleType is managing everything about val, move val's members into it; it's just doing namespacing right now and not representing an actual object.

-
Give TripleType some copy and move constructors.

TripleType(TripleType const& o) {
    type = o.type;

    switch (type) {
    case tA: a = std::make_unique(*o.a); break;
    case tB: b = std::make_unique(*o.b); break;
    case tC: c = std::make_unique(*o.c); break;
    }
}

TripleType(TripleType&& o) = default;


All told, it looks like this now:

```
#ifndef MULTITYPE_HPP
#define MULTITYPE_HPP

#include
#include

class bad_multitype_access : std::logic_err

Code Snippets

TripleType(A x) { val._e = _tA; val.__a = x; }
TripleType(B x) { val._e = _tB; val.__b = x; }
TripleType(C x) { val._e = _tC; val.__c = x; }

TripleType& operator=(A x) { val._e = _tA; val.__a = x; return *this; }
TripleType& operator=(B x) { val._e = _tB; val.__b = x; return *this; }
TripleType& operator=(C x) { val._e = _tC; val.__c = x; return *this; }
private:
    A get_(A const*) const {
        if (val._e != _tA) {
            throw bad_multitype_access(typeid(A), current_type());
        }

        return val.__a;
    }

    B get_(B const*) const {
        if (val._e != _tB) {
            throw bad_multitype_access(typeid(B), current_type());
        }

        return val.__b;
    }

    C get_(C const*) const {
        if (val._e != _tC) {
            throw bad_multitype_access(typeid(C), current_type());
        }

        return val.__c;
    }

public:
    template <class T> T get() const {
        return get_(static_cast<T const*>(nullptr));
    }
class bad_multitype_access : std::logic_error {
public:
    bad_multitype_access(std::type_info const& expected, std::type_info const& actual) :
        std::logic_error(std::string("Attempted to get ") + expected.name() + " from multitype containing " + actual.name()) {}
};
std::type_info const& current_type() const {
    switch (val._e) {
    case _tA: return typeid(A);
    case _tB: return typeid(B);
    case _tC: return typeid(C);
    }
}
private:
    enum contained_type {
        tA,
        tB,
        tC,
    };

    struct {
        A a;
        B b;
        C c;
        contained_type type;
    } val;

Context

StackExchange Code Review Q#133740, answer score: 2

Revisions (0)

No revisions yet.