12
\$\begingroup\$

I am designing a class which (among other things) acts as a vertex in an undirected graph.

#include <iterator>
#include <string>

class Node
{
public:
    explicit Node(const std::string& name);
    Node(const Node&);
    Node(Node&&);

    class iterator
        : public std::iterator<std::random_access_iterator_tag, const Node>
    {
        //...
    };
    class container_proxy
    {
    public:
        iterator begin() const;
        iterator end() const;
    private:
        //...
    };

    container_proxy neighbors() const;

    add_link(Node& other);

    //...

private:
    //...
};
// to support: for (const Node& neighbor : node.neighbors())
Node::iterator begin(const Node::container_proxy&);
Node::iterator end(const Node::container_proxy&);

Now I want to define a function link_complete which:

  • takes any number of arguments
  • allows non-const Node lvalues (parameter type Node&)
  • allows non-const Node rvalues (parameter type Node&&)
  • does not allow any other parameter type.

An example usage:

Node ret_by_value();

void example_test(Node& x, Node&& y)
{
    link_complete(x, ret_by_value(), std::move(y), Node("test"));
}

Here's the solution I came up with:

#include <initializer_list>
#include <type_traits>

class Node {
    //...
public:
    static void link_complete(std::initializer_list<Node*> node_list);
};

template <typename ... Types>
struct template_all; // UNDEFINED

template <>
struct template_all<>
  : public std::true_type {};

template <typename ... Types>
struct template_all<std::false_type, Types...>
  : public std::false_type {};

template <typename ... Types>
struct template_all<std::true_type, Types...>
  : public template_all<Types...>::type {};

template <typename ... Args>
auto link_complete(Args&& ... args) ->
  typename std::enable_if<
    template_all<
      typename std::is_same<
        typename std::remove_reference<Args>::type,
        Node
      >::type ...
    >::value,
    void
  >::type
{
    Node::link_complete( { &args... } );
}

It seems to work as required, but I'm wondering if there was a simpler or "prettier" way to do it, or maybe a way to improve that template_all helper type. All C++11, TR1, and Boost features are welcome. I did notice boost::mpl::and_ looks similar to my template_all helper type, but Boost MPL seems like a tricky learning curve and/or overkill for this problem.

\$\endgroup\$
6
+50
\$\begingroup\$

struct has public inheritance

By default, struct has public inheritance. Therefore, you don't need to manually specify it every time. You can write shorter (and so more readable) code:

template <>
struct template_all<>
  : std::true_type {};

template <typename ... Types>
struct template_all<std::false_type, Types...>
  : std::false_type {};

template <typename ... Types>
struct template_all<std::true_type, Types...>
  : template_all<Types...>::type {};

Since type traits generally use many templates, you don't want to add even more extra keywords all over the place.

Use alias templates

If you have acces to a compiler that supports some C++14 features, you will probably want to use the alias templates for transformation traits to get rid of many typename and ::type. Unfortunately, the alias templates for the query traits (is_*) have not been accepted, so you have to use std::is_same.

template <typename ... Args>
auto link_complete(Args&& ... args) ->
  std::enable_if_t<
    template_all<
      typename std::is_same<
        std::remove_reference_t<Args>,
        Node
      >...
    >::value,
    void
  >
{
    Node::link_complete( { &args... } );
}

static_assert is great

Instead of using SFINAE for link_complete, you should use static_assert since your function does not have any overload. It will allow you to give a meaningful error message instead of a somewhat obscure SFINAE error message:

template <typename ... Args>
void link_complete(Args&& ... args)
{

    static_assert(
        template_all<
            typename std::is_same<
                std::remove_reference_t<Args>,
                Node
            >::type...
        >::value,
        "link_complete arguments must be of the type Node"
    );

    Node::link_complete( { &args... } );
}

You may want to use std::decay

std::decay calls std::remove_reference and std::remove_cv internally. If needed, it also calls std::remove_extent if T is an array type and calls std::add_pointer if T is a function type. In short:

This is the type conversion applied to all function arguments when passed by value.

That allows you to check for what we generally think of as "same types":

template <typename ... Args>
void link_complete(Args&& ... args)
{

    static_assert(
        template_all<
            typename std::is_same<
                std::decay_t<Args>,
                Node
            >::type...
        >::value,
        "link_complete arguments must be of the type Node"
    );

    Node::link_complete( { &args... } );
}

Rethink template_all

All in all, your goal is to test boolean conditions on types. Since the query traits have a static constexpr bool value member, you may want to create a template_all that checks for true and false (like std::enable_if or std::conditional) instead of checking for std::true_type and std::false_type. That would make your intent clearer, but it would be more like a all_true template.

template <bool...>
struct all_true; // UNDEFINED

template <>
struct all_true<>
  : std::true_type {};

template <bool... Conds>
struct all_true<false, Conds...>
  : std::false_type {};

template <bool... Conds>
struct all_true<true, Conds...>
  : all_true<Conds...> {};

template <typename ... Args>
void link_complete(Args&& ...)
{
    static_assert(
        all_true<
            std::is_same<
                std::decay_t<Args>,
                Node
            >::value...
        >::value,
        "must have Node types"
    );

    Node::link_complete( { &args... } );
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for the detailed look. I specifically don't want to remove_cv in this case. Omitting base class access rubs my style-meter the wrong way, but that's mostly preference. I like all_true (and can't really remember why I thought using type parameters was a good idea at the time). \$\endgroup\$ – aschepler May 29 '14 at 18:54
  • \$\begingroup\$ @aschepler I tend to consider that class or struct is enough to document public/private access, but I have to admit that beginners simply don't know the difference, so explicitly specifying it isn't bad either ^^ \$\endgroup\$ – Morwenn May 29 '14 at 18:57

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.