11
\$\begingroup\$

I published a concatenation utility lastly and I feel it could be very good help for many people, but before I promote it, I would like to polish the details as much as I can. So I'll like to ask you to review this less than 300 lines of code.

#ifndef THEYPSILON_CONCAT
#define THEYPSILON_CONCAT

#include <sstream>
#include <tuple>

namespace theypsilon {

    template <typename CharT>
    struct separator_t {
        const CharT* sep;
        constexpr explicit separator_t(const CharT* s) noexcept: sep{s} {} 
    };

    template <typename CharT>
    constexpr separator_t<CharT> separator(const CharT* s) {
        return separator_t<CharT>(s);
    }

    namespace sep {
        constexpr char none [] = "";
        constexpr char space[] = " ";
        constexpr char endl [] = "\n";
        constexpr char coma [] = ", ";
        constexpr char plus [] = " + ";
    };

    namespace { // type helpers and traits
        template<typename T>
        struct has_const_iterator {
        private:
            typedef char                      yes;
            typedef struct { char array[2]; } no;

            template<typename C> static yes test(typename C::const_iterator*);
            template<typename C> static no  test(...);
        public:
            static const bool value = sizeof(test<T>(0)) == sizeof(yes);
            typedef T type;
        };

        template <typename T>
        struct has_begin_end {
            template<typename C> static char (&f(typename std::enable_if<
            std::is_same<decltype(static_cast<typename C::const_iterator (C::*)() const>(&C::begin)),
            typename C::const_iterator(C::*)() const>::value, void>::type*))[1];

            template<typename C> static char (&f(...))[2];

            template<typename C> static char (&g(typename std::enable_if<
            std::is_same<decltype(static_cast<typename C::const_iterator (C::*)() const>(&C::end)),
            typename C::const_iterator(C::*)() const>::value, void>::type*))[1];

            template<typename C> static char (&g(...))[2];

            static bool const beg_value = sizeof(f<T>(0)) == 1;
            static bool const end_value = sizeof(g<T>(0)) == 1;
        };

        template<typename T, typename CharT>
        constexpr bool is_writable_stream() {
            return  std::is_same<T, std::basic_ostringstream<CharT>>::value || 
                    std::is_same<T, std::basic_stringstream <CharT>>::value ||
                    std::is_same<T, std::basic_ostream<CharT>>::value;
        }

        template<typename T, typename CharT = char> 
        constexpr bool is_stringstream() {
            return  std::is_same<T, std::basic_istringstream<CharT>>::value || 
                    std::is_same<T, std::basic_ostringstream<CharT>>::value ||
                    std::is_same<T, std::basic_stringstream <CharT>>::value;
        }

        template<typename T> 
        constexpr bool is_char_type() {
            return  std::is_same<typename std::decay<T>::type, char    >::value || 
                    std::is_same<typename std::decay<T>::type, wchar_t >::value || 
                    std::is_same<typename std::decay<T>::type, char16_t>::value || 
                    std::is_same<typename std::decay<T>::type ,char32_t>::value;
        }

        template<typename T, typename CharT = char>
        constexpr bool is_c_str() { 
            return  std::is_same<typename std::decay<T>::type, CharT const *>::value ||
                    std::is_same<typename std::decay<T>::type, CharT       *>::value;
        }

        template<typename T> 
        constexpr bool is_char_sequence() {
            return  is_c_str<T,     char>() ||
                    is_c_str<T,  wchar_t>() ||
                    is_c_str<T, char16_t>() ||
                    is_c_str<T, char32_t>();
        }

        template<typename T> 
        constexpr bool is_container() {
            return (has_const_iterator<T>::value && 
                    has_begin_end<T>::beg_value  && 
                    has_begin_end<T>::end_value  &&
                    !std::is_same<T, std::string>::value &&
                    !is_stringstream<T>()) 
            || (std::is_array<T>::value && !is_char_sequence<T*>());
        }

        template <typename T>
        constexpr bool is_string() {
            return  std::is_same<T,std::string   >::value ||
                    std::is_same<T,std::wstring  >::value ||
                    std::is_same<T,std::u16string>::value ||
                    std::is_same<T,std::u32string>::value;
        }

        template <typename T>
        constexpr bool is_basic_type() {
            return std::is_scalar<T>::value || is_string<T>();
        }

        template <typename T, template <typename...> class Template>
        struct is_specialization_of : std::false_type {};

        template <template <typename...> class Template, typename... Args>
        struct is_specialization_of<Template<Args...>, Template> : std::true_type {};

        template <typename T>
        constexpr bool is_modifier() {
            return  !is_container    <T>() && !is_stringstream    <T>() && 
                    !is_char_sequence<T>() && !is_basic_type<T>() &&
                    !std::is_array<T>::value && 
                    !is_specialization_of<T, std::tuple>::value;
        }
    }

    namespace { // concat_intern

        template <typename CharT, char head, char... tail>
        std::basic_string<CharT> get_separator() { return {head, tail...}; }

        template <typename W, typename S>
        void separate(W& writter, const S* separator) { 
            if (separator) writter << separator;
        }

        template <typename W, typename S>
        void separate(W& writter, const S& separator) { 
            writter << separator;
        }

        template <typename CharT, typename W>
        std::basic_string<CharT> concat_to_string(const W& writter) {
            return writter.good() ? writter.str() : std::basic_string<CharT>();
        }

        template <typename CharT, typename W, typename S, typename T>
        void concat_intern_write(W&, const S&, bool, const T&);

        template <typename CharT, typename W, typename S, typename T>
            typename std::enable_if<is_char_sequence<T*>(),
        void>::type concat_intern_recursion(W& writter, const S& separator, const T* v) {
            if (v) writter << v;
        }

        template <typename CharT, typename W, typename S, typename T>
            typename std::enable_if<
                (!is_container<T>() && !is_stringstream<T>() && !is_char_sequence<T>()) || is_modifier<T>(),
        void>::type concat_intern_recursion(W& writter, const S& separator, const T& v) {
            writter << v;
        }

        template <typename CharT, typename W, typename S, typename T>
            typename std::enable_if<is_stringstream<T>(),
        void>::type concat_intern_recursion(W& writter, const S& separator, const T& v) {
            if (v.good()) writter << concat_to_string<CharT>(v);
            else writter.setstate(v.rdstate());
        }

        template <typename CharT, typename W, typename S, typename T>
            typename std::enable_if<is_container<T>(),
        void>::type concat_intern_recursion(W& writter, const S& separator, const T& container) {
            auto it = std::begin(container), et = std::end(container);
            while(it != et) {
                auto element = *it;
                it++;
                concat_intern_write<CharT>(writter, separator, it != et, element);
            }
        }

        template<unsigned N, unsigned Last>
        struct tuple_printer {
            template<typename CharT, typename W, typename S, typename T>
            static void print(W& writter, const S& separator, const T& v) {
                concat_intern_write<CharT>(writter, separator, true, std::get<N>(v));
                tuple_printer<N + 1, Last>::template print<CharT>(writter, separator, v);
            }
        };

        template<unsigned N>
        struct tuple_printer<N, N> {
            template<typename CharT, typename W, typename S, typename T>
            static void print(W& writter, const S& separator, const T& v) {
                concat_intern_write<CharT>(writter, separator, false, std::get<N>(v));
            }
        };

        template <typename CharT, typename W, typename S, typename... Args>
        void concat_intern_recursion(W& writter, const S& separator, const std::tuple<Args...>& v) {
            tuple_printer<0, sizeof...(Args) - 1>::template print<CharT>(writter, separator, v);
        }

        template <typename CharT, typename W, typename S, typename T, typename... Args>
        void concat_intern_recursion(W& writter, const S& separator, const T& head, const Args&... tail) {
            concat_intern_write<CharT>(writter, separator, true, head);
            concat_intern_recursion<CharT>(writter, separator, tail...);
        }

        template <typename CharT, typename W, typename S, typename T>
        inline void concat_intern_write(W& writter, const S& separator, bool b, const T& v) {
            concat_intern_recursion<CharT>(writter, separator, v);
            if (b && !is_modifier<T>()) separate(writter, separator);
        }

        template <typename CharT, typename S, typename T, typename... Args,
            typename = typename std::enable_if<is_writable_stream<T, CharT>() == true, T>::type>
        std::basic_string<CharT> concat_intern(const S& separator, T& writter, const Args&... seq) {
            concat_intern_recursion<CharT>(writter, separator, seq...);
            return concat_to_string<CharT>(writter);
        }

        template <typename CharT, typename S, typename... Args>
        std::basic_string<CharT> concat_intern(const S& separator, const Args&... seq) {
            std::basic_ostringstream<CharT> writter;
            return concat_intern<CharT>(separator, writter, seq...);
        }
    }

    template <typename CharT = char, typename... Args>
    std::basic_string<CharT> concat(const separator_t<CharT>& sep, Args&&... seq) {
        return concat_intern<CharT>(
            sep.sep, 
            std::forward<Args>(seq)...
        );
    }

    template <char head, char... tail, typename F, typename... Args,
        typename = typename std::enable_if<std::is_same<F, separator_t<char>>::value == false, F>::type>
    std::basic_string<char> concat(F&& first, Args&&... rest) {
        return concat_intern<char>(
            get_separator<char, head, tail...>(), 
            std::forward<F>(first), 
            std::forward<Args>(rest)...
        );
    }

    template <const char* sep, typename F, typename... Args,
        typename = typename std::enable_if<std::is_same<F, separator_t<char>>::value == false, F>::type>
    std::basic_string<char> concat(F&& first, Args&&... rest) {
        return concat_intern<char>(
            sep, 
            std::forward<F>(first), 
            std::forward<Args>(rest)...
        );
    }

    template <typename CharT = char, typename F, typename... Args,
        typename = typename std::enable_if<std::is_same<F, separator_t<CharT>>::value == false, F>::type>
    std::basic_string<CharT> concat(F&& first, Args&&... rest) {
        return concat_intern<CharT>(
            (const CharT*)nullptr, 
            std::forward<F>(first), 
            std::forward<Args>(rest)...
        );
    }

    template <std::ostream& sep (std::ostream&), typename CharT = char, typename F, typename... Args,
        typename = typename std::enable_if<std::is_same<F, separator_t<CharT>>::value == false, F>::type>
    std::basic_string<CharT> concat(F&& first, Args&&... rest) {
        return concat_intern<CharT>(
            sep, 
            std::forward<F>(first), 
            std::forward<Args>(rest)...
        );
    }

}

#endif
  • Does this code follow the common conventions?
  • Do you think there is any way to improve this implementation in terms of efficiency, versatility or convenience?
  • Do you think this could also benefit from new changes of C++14?

This would be sample of the client code:

std::cout << concat(1,2,3,4,5) << std::endl; // prints "12345"

But is really much more versatile than this, you can check more here.

\$\endgroup\$
  • 3
    \$\begingroup\$ Welcome to Code Review! I love it when new users ask such nice questions. \$\endgroup\$ – Edward Jul 27 '14 at 20:43
  • \$\begingroup\$ Small thing caught my eye: there's a typo of coma that should be comma. \$\endgroup\$ – Corbin Jul 28 '14 at 2:58
7
\$\begingroup\$

The only advice I could offer is to use "modern" style traits that work nicely with the standard library through the usage of std::integral_constant and expression SFINAE.

For example, has_const_iterator can be rewritten like this:

template<typename...>
struct void_ {
    using type = void;
};

template<typename... Args>
using Void = typename void_<Args...>::type;

template<typename T, typename U = void>
struct has_const_iterator : public std::false_type {};

template<typename T>
struct has_const_iterator<T, Void<typename T::const_iterator>> : public std::true_type {};

Now you get the benefit of std::integral_constant without having to redefine ::value. It'll also work with tag dispatching that takes in std::true_type or std::false_type as parameters.

It's fairly uncommon to define traits as constexpr bool functions as well since this disallows the use of higher ordered meta functions, so I'd suggest moving those into their own traits like you did with has_begin_end and has_const_iterator.

On a final note, has_begin_end can be simplified immensely through the use of expression SFINAE as seen here:

struct has_begin_end_impl {
    template<typename T, typename B = decltype(std::declval<T&>().begin()),
                         typename E = decltype(std::declval<T&>().end())>
    static std::true_type test(int);
    template<typename...>
    static std::false_type test(...);
};

template<typename T>
struct has_begin_end : public decltype(has_begin_end_impl::test<T>(0)) {};

You can learn more about expression SFINAE here. As for the reason that there's test(int) and test(...), that's for making the call test(0) unambiguous.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot! I updated the code with your feedback in my repo. Do you know if is possible to use SFINAE to detect if a non-member free function is overloaded for a given class? (like std::begin) \$\endgroup\$ – José Manuel Jul 28 '14 at 16:00
  • 1
    \$\begingroup\$ @JoséManuel Yup. Just do the same expression SFINAE trick above, i.e. decltype(std::begin(std::declval<T&>())). \$\endgroup\$ – Rapptz Jul 28 '14 at 19:44
3
\$\begingroup\$

In all, this is a nice piece of work. I have only a few minor points.

Remove formal name separarator to quiet compiler warnings

When I compile the code with g++ with maximum warning levels, it complains that the parameter separator is not used in several functions. Tracing this, it appears that a number of the concat_intern_recursion functions don't use it, so in my local copy, I've removed the formal parameter name separator (that is, I just leave const S&) from those calls and now get no warnings.

Fix the spelling of writer

The word is spelled as writter throughout the code and documentation, but the correct spelling is writer with a single t.

Consider if constexpr could be supported

I'm not sure if it would be possible to do, but the one thing I tried to do that it didn't support was usage like this:

constexpr std::string foo = concat(separator(" < "),1,3,5);

The compiler said it was due to the fact that std::string has a non-trivial destructor so perhaps it's out of your hands, but something to think about.

\$\endgroup\$
  • 1
    \$\begingroup\$ There is a way to do string concatenation with constexpr but this is only possible in C++14. std::string however wouldn't support it, you'd need a specific constexpr string type. It's fun to write, if anything \$\endgroup\$ – Rapptz Jul 28 '14 at 0:35
  • \$\begingroup\$ Here is a proposal for a standard constexpr string class. You may want to use it as a base. \$\endgroup\$ – Morwenn Jul 28 '14 at 13:57
  • \$\begingroup\$ Thanks for the corrections. constexpr would be nice, I'm also looking forward to string literals to pass the separator as a template argument in a nicer way. \$\endgroup\$ – José Manuel Jul 28 '14 at 16:01

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.