6
\$\begingroup\$

I have decided to write my own min max sum functions. I really don't like the ones provided by the STL so I decided to write my own.

#include <type_traits>
namespace utils {
namespace detail {
    template<typename T1, typename T2>
    using BiggerT = typename std::conditional<sizeof(T1) >= sizeof(T2), T1, T2>;
    template<typename T1, typename T2>
    using FloatingPointOverBiggerT = typename std::conditional < std::is_floating_point<T1>::value || std::is_floating_point<T2>::value,
        double, typename BiggerT<T1, T2>>;

    template<typename...> struct BiggestType;

    template<typename T1, typename T2, typename... RestT>
    struct BiggestType<T1, T2, RestT...> {
        using type = typename FloatingPointOverBiggerT<T1, typename BiggestType<T2, RestT...>::type>::type;
    };

    template<typename T>
    struct BiggestType<T> { 
        using type = T; 
    };

    template<typename T1, typename T2>
    struct BiggestType<T1, T2>{ 
        using type = typename FloatingPointOverBiggerT<T1, T2>::type;
    };

    template<typename FirstNumericT, typename SecondNumericT>
    constexpr decltype(auto) Max(FirstNumericT&& first, SecondNumericT&& second) noexcept {
        using Type = typename detail::BiggestType<FirstNumericT, SecondNumericT>::type;
        return (static_cast<Type>(first) > static_cast<Type>(second)) ? static_cast<Type>(first) : static_cast<Type>(second);
    }

    template<typename FirstNumericT, typename SecondNumericT, typename... RestNumericT>
    constexpr decltype(auto) Max(FirstNumericT&& first, SecondNumericT&& second, RestNumericT&&... restargs) noexcept {
        using Type = typename detail::BiggestType<FirstNumericT, SecondNumericT, RestNumericT...>::type;
        return Max((static_cast<Type>(first) > static_cast<Type>(second)) ?
            static_cast<Type>(first) : static_cast<Type>(second), std::forward<RestNumericT&&>(restargs)...);
    }

    template<typename FirstNumericT, typename SecondNumericT>
    constexpr decltype(auto) Min(FirstNumericT&& first, SecondNumericT&& second) noexcept {
        using Type = typename detail::BiggestType<FirstNumericT, SecondNumericT>::type;
        return (static_cast<Type>(first) < static_cast<Type>(second)) ? static_cast<Type>(first) : static_cast<Type>(second);
    }

    template<typename FirstNumericT, typename SecondNumericT, typename... RestNumericT>
    constexpr decltype(auto) Min(FirstNumericT&& first, SecondNumericT&& second, RestNumericT&&... restargs) noexcept {
        using Type = typename detail::BiggestType<FirstNumericT, SecondNumericT, RestNumericT...>::type;
        return Min((static_cast<Type>(first) < static_cast<Type>(second)) ?
            static_cast<Type>(first) : static_cast<Type>(second), std::forward<RestNumericT&&>(restargs)...);
    }

    template<typename LastNumericT>
    constexpr decltype(auto) Sum(LastNumericT last) {
        return last;
    }

    template<typename FirstNumericT, typename SecondNumericT, typename... RestNumericT>
    constexpr decltype(auto) Sum(FirstNumericT&& first, SecondNumericT&& second, RestNumericT&&... restargs) noexcept {
        using Type = typename detail::BiggestType<FirstNumericT, SecondNumericT, RestNumericT...>::type;
        return Sum(static_cast<Type>(first) + static_cast<Type>(second), std::forward<RestNumericT&&>(restargs)...);
    }

    template < typename... >
    using void_t = void;
    template < typename T, typename Index >
    using SubscriptT = decltype(std::declval<T>()[std::declval<Index>()]);
    template < typename, typename Index = size_t, typename = void_t<> >
    struct HasSubscriptOperator : std::false_type {};
    template < typename T, typename Index >
    struct HasSubscriptOperator< T, Index, void_t< SubscriptT<T, Index> > > : std::true_type {};

}

template<typename ReturnT = int, typename ContainerT, typename PredicateT, typename RawT =
typename std::remove_cv<typename std::remove_reference<ReturnT>::type>::type>
constexpr typename std::enable_if < utils::detail::HasSubscriptOperator<ContainerT>::value, RawT>::type
Max(const ContainerT& container, const PredicateT& predicate) {
    auto result = predicate(*std::cbegin(container));
    for(const auto& num : container) {
        if(predicate(num) > result) {
            result = predicate(num);
        }
    }
    return result;
}

template<typename FirstT, typename SecondT, typename... NumericTypes>
constexpr typename std::enable_if < std::is_floating_point<FirstT>::value || std::is_integral<FirstT>::value,
typename utils::detail::BiggestType<FirstT, SecondT, NumericTypes...>::type>::type
Max(FirstT&& first, SecondT&& second, NumericTypes&&... args) noexcept {
    return detail::Max(std::forward<FirstT&&>(first), std::forward<SecondT&&>(second), std::forward<NumericTypes&&>(args)...);
}

template<typename ContainerT>
constexpr decltype(auto) Max(const ContainerT& container) {
    typename std::remove_cv<typename std::remove_reference<decltype(container[0])>::type>::type result = container[0];
    for(const auto& num : container) {
        if(num > result) {
            result = num;
        }
    }
    return result;
}

template<typename FirstT, typename SecondT, typename... NumericTypes>
constexpr typename std::enable_if < std::is_floating_point<FirstT>::value || std::is_integral<FirstT>::value,
typename utils::detail::BiggestType<FirstT, SecondT, NumericTypes...>::type>::type  
Min(FirstT&& first, SecondT&& second, NumericTypes&&... args) noexcept {
    return detail::Min(std::forward<FirstT&&>(first), std::forward<SecondT&&>(second), std::forward<NumericTypes&&>(args)...);
}

template<typename ReturnT = int, typename ContainerT, typename PredicateT, typename RawT =
typename std::remove_cv<typename std::remove_reference<ReturnT>::type>::type>
constexpr typename std::enable_if < utils::detail::HasSubscriptOperator<ContainerT>::value, RawT>::type
Min(const ContainerT& container, const PredicateT& predicate) {
    auto result = predicate(*std::cbegin(container));
    for(const auto& num : container) {
        if(predicate(num) < result) {
            result = predicate(num);
        }
    }
    return result;
}

template<typename ContainerT>
constexpr decltype(auto) Min(const ContainerT& container) {
    typename std::remove_cv<typename std::remove_reference<decltype(container[0])>::type>::type result = container[0];
    for(const auto& num : container) {
        if(num < result) {
            result = num;
        }
    }
    return result;
}

template<typename ReturnT = int, typename ContainerT, typename PredicateT, typename RawT =
typename std::remove_cv<typename std::remove_reference<ReturnT>::type>::type>
constexpr typename std::enable_if < utils::detail::HasSubscriptOperator<ContainerT>::value, RawT>::type
Sum(const ContainerT& container, PredicateT predicate) {
    RawT result = 0;
    for(const auto& num : container) {
        result += predicate(num);
    }
    return result;
}

template<typename FirstT, typename SecondT, typename... NumericTypes>
constexpr typename std::enable_if < std::is_floating_point<FirstT>::value || std::is_integral<FirstT>::value, 
typename utils::detail::BiggestType<FirstT, SecondT, NumericTypes...>::type>::type 
Sum(FirstT&& first, SecondT&& second, NumericTypes&&... args) noexcept {
    return detail::Sum(std::forward<FirstT&&>(first), std::forward<SecondT&&>(second), std::forward<NumericTypes&&>(args)...);
}

template<typename ContainerT>
constexpr decltype(auto) Sum(const ContainerT& container) {
    typename std::remove_cv<typename std::remove_reference<decltype(container[0])>::type>::type result = 0;
    for(const auto& num : container) {
        result += num;
    }
    return result;
}

template<typename FirstT, typename SecondT, typename... NumericTypes>
constexpr decltype(auto) Avg(FirstT&& first, SecondT&& second, NumericTypes&&... args) noexcept {
    return (detail::Sum(std::forward<FirstT&&>(first), std::forward<SecondT&&>(second), std::forward<NumericTypes&&>(args)...) / (2 + sizeof...(NumericTypes)));
}

Usage

std::vector<int> vecInt = {2, 3, 5, 2, 10};
    std::map<int, std::size_t> mapInt = {{2, 2}, {3, 3}, {7, 7}};
    int cInt[] = {2, 2, 2, 2};
    auto vecSum = utils::Sum(vecInt);
    auto mapSum = utils::Sum(mapInt, [](const auto& val) {return val.second;});
    auto cSum = utils::Sum(cInt);
    auto sum = utils::Sum(5, 7ULL, 5.5f, 70.0);
    auto vecMax = utils::Max(vecInt);
    auto mapMax = utils::Max(mapInt, [](const auto& val) {return val.second;});
    auto cMax = utils::Max(cInt);
    auto maxx = utils::Max(5, 55, 11, 33.0f, 55.0, 66ULL);
    auto vecMin = utils::Min(vecInt);
    auto mapMin = utils::Min(mapInt, [](const auto& val) {return val.second;});
    auto cMin = utils::Min(cInt);
    auto minn = utils::Min(5, 55, 11, 33.0f, 55.0, 66ULL);
    auto avgg = utils::Avg(5, 55, 11, 33.0f, 55.0, 66ULL);

What do you think about the code?

\$\endgroup\$
  • 8
    \$\begingroup\$ Although not strictly necessary, it would be useful to reviewers if you could explain why you don't care for the STL versions and how you think this version is better. \$\endgroup\$ – Edward Apr 15 '18 at 21:52
  • \$\begingroup\$ This code doesn't compile: you need to #include <iterator> and to remove the typename in front of BiggerTat line 8 (edit: you should rather add ::typeafter that). Your usage example should also #include <vector> /*and*/ <map> and be expanded with some output to make behavior more explicit. \$\endgroup\$ – papagaga Apr 16 '18 at 7:00
4
\$\begingroup\$

Is it intended?

It really is difficult to see what you're trying to achieve here. Your code prevents some basic usage of a max function, for instance:

unsigned int u = 5u; 
int i = 7; 
std::cout << "max(" << u << ", " << i << ") = " << utils::Max(u, i);

results in:

prog.cc:35:44: error: non-const lvalue reference to type 'unsigned int' cannot bind to a value of unrelated type 'int' return (static_cast(first) > static_cast(second)) ? static_cast(first) : static_cast(second); ^ ~~~~~~

prog.cc:80:63: note: in instantiation of function template specialization 'utils::Max' requested here std::cout << "max(" << u << ", " << i << ") = " << utils::Max(u, i); ^

prog.cc:35:100: error: non-const lvalue reference to type 'unsigned int' cannot bind to a value of unrelated type 'int' return (static_cast(first) > static_cast(second)) ? static_cast(first) : static_cast(second); ^ ~~~~~~ 2 errors generated.

This means that you can't use the 'numeric' overload but with literal values, be it as arguments or as return type. That is really restrictive. If I want to decrement the highest value of two, I can't, for instance.

At this point, I'm not sure whether it's intended behavior, or if you should fix your code before submitting again.

Hypothetical review

Since I don't know if you want to keep your code like this, I won't dwell upon it too long. But there are a few key things you should consider:

  • commenting your code a bit: good comments these days are about intent, and this is something your readers would benefit from.

  • <limits> is, I think, what you're looking for when comparing types. sizeof doesn't tell you a thing about the range of values a type can represent. Think of what you should do when comparing int and unsigned int, which generally are the same size: the selected type shouldn't be the same if you call min or max...

  • max and max_element are separated in the standard library. Why would you merge them? Overloading has always been a tricky business, and along with templates makes for unreadable error messages, so don't do it unless there's a good reason to.

  • there are containers that don't provide operator[]. An std::list is out of the scope of your Max function for an obscure reason. And the semantics for a std::map could surprise too, since anyone with basic knowledge of the STL considers const std::pair<Key, T> to be the elements' type.

\$\endgroup\$
1
\$\begingroup\$

There is an incredible amount of boilerplate here. I would like to remind you that with C++17 you can use fold expressions, which do basically the same but way simpler:

template<typename ... T>
auto sum(T ... t) {
    return (t + ...);
}

If you want to be sure that an empty sum will also compile you can do this:

template<typename ... T>
auto sum(T ... t) {
    return (t + ... + 0);
}

You can obviously expand that to different types etc

\$\endgroup\$

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.