1
\$\begingroup\$

I tried to find a generic way to deal with associations. I ended up with this code which seems to fit my needs:

#include <iostream>
#include <tuple>
#include <set>
#include <string>

template <typename D, typename... Ts>
class Relations;

namespace std
{
    template <std::size_t I, typename D, typename... Ts>
    auto get(Relations<D, Ts...>& r) -> typename std::tuple_element<I, std::tuple<Ts...>>::type&;
}

template <typename A1, typename A2, size_t ind2>
class Association
{
public:
    ~Association()
    {
        if (!this->empty())
        {
            this->clear_associations();
        }
    }

    void associate(A2* ref)
    {
        if (!this->empty() && _ref == ref)
        {
            return;
        }

        if (_ref)
        {
            std::get<ind2>(*_ref).reset_association();
        }
        _ref = ref;
        std::get<ind2>(*ref).associate(static_cast<A1*>(this));
    };

    void associate(A2& ref)
    {
        this->associate(&ref);
    };

    bool empty() const
    {
        if (_ref==0)
            return true;
        else
            return false;
    }

    void remove_association(A2* ref)
    {
        if (_ref == ref)
        {
            this->reset_association();
            std::get<ind2>(*ref).remove_association(static_cast<A1*>(this));
        }
    }

    void remove_association(A2& ref)
    {
        this->remove_association(&ref);
    }

    void reset_association()
    {
        _ref = 0;
    }

    void clear_associations()
    {
        if (_ref)
        {
            std::get<ind2>(*_ref).remove_association(static_cast<A1*>(this));
        }
        this->reset_association();
    }

    bool has_association(A2* ref) const
    {
        if (this->_ref == ref)
            return true;
        else
            return false;
    }

    bool has_association(A2& ref) const
    {
        return this->has_association(&ref);
    }

    A2& get_associated() const
    {
        return *_ref;
    }

private:
    A2* _ref=0;
};

template <typename D, typename... Ts>
class Relations : public Ts...
{
public:
    Relations() {};
    ~Relations() {};

    template <size_t N>
    auto get_associated()
    {
        return std::get<N>(*this).get_associated();
    };

    template <size_t N>
    auto& get_association()
    {
        return std::get<N>(*this);
    };

    template <size_t N, typename T>
    void associate(T& j)
    {
        std::get<N>(*this).associate(j);
    };

    template <size_t N, typename T>
    void dissociate(T& j)
    {
        std::get<N>(*this).remove_association(j);
    };
};

namespace std
{
    template <std::size_t I, typename D, typename... Ts>
    auto get(Relations<D, Ts...>& r) -> typename std::tuple_element<I, std::tuple<Ts...>>::type&
    {
        return static_cast<typename std::tuple_element<I, std::tuple<Ts...>>::type&>(r);
    }
}

class I;
class J;
class K;

class I : public Relations<I, Association<I, J, 0>, Association<I, K, 0>>
{
public:
    std::string type="I";
    auto& J_asso()
    {
        return this->get_association<0>();
    }
    auto& K_asso()
    {
        return this->get_association<1>();
    }
};

class J : public Relations<J, Association<J, I, 0>>
{
public:
    std::string type="J";
    auto& I_asso()
    {
        return this->get_association<0>();
    }
};

class K : public Relations<K, Association<K, I, 1>>
{
public:
    std::string type="K";
    auto& I_asso()
    {
        return this->get_association<0>();
    }
};

int main()
{
    I i;
    J j;
    K k;
    i.J_asso().associate(j);
    i.K_asso().associate(k);
    std::cout<<i.type<<";"<<i.J_asso().get_associated().type<<std::endl;
    std::cout<<i.type<<";"<<i.K_asso().get_associated().type<<std::endl;

    i.K_asso().get_associated().type="new K";
    std::cout<<i.type<<";"<<i.K_asso().get_associated().type<<std::endl;

    return 0;
}

My questions about this code are:

  • I am not fully satisfied with the interface but I don't really know how to make it better. Right now I have to define the association in template and then add a member to make it more readable. Is it possible to make something simpler? Any idea or feedback on this subject would be nice.
  • Does this code seems easy to maintain to you? And easy to update? I never had to do industrial code so I do not really know what are the standards...
  • I did not find any library to do this while it seems to me that it could be often useful. Is there any existing lib that I did not find? If there is not, why?

I will later dissociate definitions and implementations of member functions but only when I am satisfied with them.

I also implemented a multi-association class in the same way so that I can can either pass Association or MultiAssociation in template. I hid it to be more concise.

\$\endgroup\$
  • 1
    \$\begingroup\$ What problem are you trying to solve? What are these relations and associations good for? (What do they model?) I read your post twice and I don't understand :( \$\endgroup\$ – utnapistim Aug 26 '15 at 8:24
  • \$\begingroup\$ I have many classes and I want to relate (relationships are associations) some objects of a type to some objects of another type. I want to have a generic code to define this kind of relations so I do not have to define a pointer in each class then always the same set of operations on this pointer (associate, dissociate, get the associated object, etc.). This is what this code do. But now I would like to be sure that this code is clean, maintainable and respects some standards (I am from the academic world where the standards are very different). I am also aware of any other way to do this :) \$\endgroup\$ – StormRider Aug 26 '15 at 14:22
4
\$\begingroup\$

Naming should describe operation

I have no idea what this code is trying to achieve (that's a bad sign).

Naming types A1 and A2 when they have some deterministic meaning (that I don't understand) is confusing. Normal I go for very short types names myself ('T' for example). But only when the type does not matter. Here I feel the type does matter.

But this code definitely needs some heavy commenting on what it is trying to achieve. As written this is un-maintainable because any maintainer will struggle to work out what is happening.

std is sacrosanct

This is illegal:

namespace std
{
    template <std::size_t I, typename D, typename... Ts>
    auto get(Relations<D, Ts...>& r) -> typename std::tuple_element<I, std::tuple<Ts...>>::type&;
}

You are not allowed to add stuff to the standard. You will break things. Especially with a name like get() Put this in your own namespace. Koenig lookup will correctly deduce the namespace of your function.

Avoid the use of this

If you have to use this it means you have written un-maintainable code because you are having to explicitly tell the compiler what to do because you have several names that collide and you are trying to differentiate between them.

~Association()
{
    if (!this->empty())
    {
        this->clear_associations();
    }
}

Turn on your compiler warnings (and treat them as errors) then you can resolve shadowed names with better ones and avoid the trap of using this->.

Avoid if then else to check state of a bool or pointer.

bool empty() const
{
    if (_ref==0)
        return true;     // If the above expr is true  return true
    else
        return false;    // If the above expr is false return false
}

Much easier to read and write as:

 bool empty() const  {return _ref == 0;} // just return the expr.
\$\endgroup\$
  • \$\begingroup\$ Coming back to the std::get question. While a namespace-level one wouldn't be found by ADL, I re-read the question and realized that what we have here is an overload of std::get for a user-defined type, not a specialization. So... it is probably not legal in the end; you might want to restore a bit of advice :) \$\endgroup\$ – Morwenn Aug 27 '15 at 12:05
  • \$\begingroup\$ Thank you very much for your comment! I have no time until next week to study it properly but I will come back :) \$\endgroup\$ – StormRider Sep 1 '15 at 15:24
  • \$\begingroup\$ Thank you very much for these tips :) I agree with all except the one on std. As it is specific to my own class, how could it break anything? And if I move it to another namespace, wouldn't it be less convenient to use? (I think there are some STL functions that use std::get by default, right?) \$\endgroup\$ – StormRider Sep 8 '15 at 15:50
  • \$\begingroup\$ @user52730: Weather you agree or not is irrelevant. What you are doing is illegal and thus undefined behavior. Undefined behavior means your code may or may not work as expected depending on the whims on the system. Use at your own risk but don't expect it to be portable or maintainable. \$\endgroup\$ – Martin York Sep 8 '15 at 18:19
  • \$\begingroup\$ Well, 'agree' might not be the right term, sorry =) What I mean is that is seems safe to me but I am sure you are right, I just don't see a case where it is not safe. \$\endgroup\$ – StormRider Sep 8 '15 at 19:20
2
\$\begingroup\$

Right now I have to define the association in template and then add a member to make it more readable. Is it possible to make something simpler?

Yes. You can remove the difference between association and relationship, and define a single concept ("connectable object").

Does this code seems easy to maintain to you? And easy to update?

It was a bit difficult to read. Here are some suggestions:

  • write the public interface of your class after writing two blocks of client code for it (if possible, performing different functions). You can turn those into unit tests later, or discard them at the end. You should also use this to name your entities and operations (this is how I came up with the names in the code below).

  • follow the "tell, don't ask" principle: client code should not inquire into the properties of an object, then take decisions; instead, they should tell objects what to do;

I did not find any library to do this while it seems to me that it could be often useful. Is there any existing lib that I did not find? If there is not, why?

There shouldn't be a library, because it is easy to implement (as a single base class template):

/// @brief base class for connected objects
///
template<typename From, typename To>
class connected
{
    To *ptr = nullptr;

    friend class T;        
    void non_recursive_connect(To& t)
    {
        disconnect();
        ptr = &t;
    }
public:
    virtual ~connected() = default;

    void connect(To& t)
    {
       t.non_recursive_connect(static_cast<From&>(*this));
       non_recursive_connect(t);
    }

    // template function not really necessary; only used to
    // enforce template function syntax, matching connected<Tn...>
    // in signature
    template<typename Other, class = typename std::enable_if<
           std::is_same<Other, To>::value
        >::type>
    void disconnect()
    {
        if(ptr)
        {
            auto p = ptr;
            ptr = nullptr; // reset pointer before calling into
                // disconnect, to avoid stack overflow
            p->disconnect();
        }
    }

    // template function not really necessary; only used to
    // enforce template function syntax, matching connected<Tn...>
    // in signature
    template<typename Other, class = typename std::enable_if<
           std::is_same<Other, To>::value
        >::type>
    Other& get_connected()
    {
        if(!ptr)
            throw std::runtime_error{ "connected<From,To>::get_connected:"
                "not connected");
        return *ptr;
    }
};

Create a new base, for objects connected to more than one object, that offers the same interface to client code:

// enable template if T is any of the rest of the types
template <typename T, typename Head, typename ...Rest>
struct is_any_of<T, Head, Rest...>
: std::integral_constant<bool, std::is_same<T, Head>::value ||
                           is_any_of<T, Rest...>::value>
{ };

/// @brief base class for objects connected to more than one other type
template<typename From, typename ...Tn>
class connected: connected<From,Tn>, ...
{
    template<typename T,
        class = typename std::enable_if<
           is_any_of<T, Tn...>::value
        >::type>>
    auto base()
    {
        return static_cast<connected<From,T>&>(*this);
    }
public:
    virtual ~connected() = default;

    template<typename T>
    void connect(T& t) { return base<T>().connect(t); }

    template<typename T>
    auto get_connected() { return base<T>().get_connected<T>(); }

    template<typename T>
    void disconnect() { base<T>().disconnect<T>(); }
};

Client classes:

class I: public connected<I, J, K /* add more here */ > { ... };
struct J: connected<J, I> { ... };
struct K: connected<K, I> { ... };

Client code:

I i;
J j;
K k;

i.connect(j);
i.connect(k);

assert(k.get_connected<I>() == i);

j.get_connected<K>(); // will fail to compile (they are not connected)
k.get_connected<I>();

i.disconnect<K>();
i.get_connected<K>(); // will throw runtime_error
k.get_connected<I>(); // will throw runtime_error

The code probably won't compile as it is, but it shows (I hope) a simpler approach.

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your comment! I have no time until next week to study it properly but I will come back :) \$\endgroup\$ – StormRider Sep 1 '15 at 15:21
  • \$\begingroup\$ I finally have some time to work on this topic. I really like your solution but I have a question: is it possible to have two different relations to the same type with your solution? So I can write class I: public connected<I, J, J, K>. It seems to me that it is not possible but I am not sure (and I think I will need this feature, that is why I had an index in template in my code but I guess there are better ways to do this). Thanks again! \$\endgroup\$ – StormRider Sep 8 '15 at 14:37
  • \$\begingroup\$ You can store a vector of pointers in each connected implementation I guess ... (but as it is, no - you cannot repeat types). \$\endgroup\$ – utnapistim Sep 8 '15 at 17:57
  • \$\begingroup\$ Ok, thank you. As I said in my first post, I also implemented a MultiAssociation which is basically a set of pointers. So I would like to have the choice between Association or MultiAssociation AND the possibility to store different relations to the same type, that is why my solution was more complicated than yours. But some parts of your code are very enlightening to me so thanks again! \$\endgroup\$ – StormRider Sep 9 '15 at 9:38

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.