My first (simple) template that does more than storing a value of type T

Question

I just decided to get a bit more acustomed to using templates. Maybe this is a stupid question, but in my experience in C++ there is always some pitfall lurking aroung the corner. Thus I prefer the danger of embarresment instead of building up on bad habbits. Mainly as an exercise I wrote this:

#include <functional>
#include <string>
#include <sstream>

template <typename V,template <class> class R = std::greater,typename I=void>
struct Extremum {
    typedef V    value_type;
    typedef I    index_type;
    typedef R<V> relation_type;
    V value;
    I index;
    R<V> r;
    bool operator() (V v,I i){
        bool result = r(v,value);
        if (result){
            value = v;
            index = i;
        }
        return result;
    }
    Extremum(V v,I i) : value(v),index(i) {}
    std::string toString(){
        std::stringstream s;
        s << "value= " << value << " index= " << index;
        return s.str();
    }
};

template <typename V,template <class> class R>
struct Extremum<V,R,void> {
    typedef V    value_type;
    typedef void index_type;
    typedef R<V> realtion_type;
    V value;
    R<V> r;
    bool operator() (V v){
        bool result = r(v,value);
        if (result){ value = v; }
        return result;
    }
    Extremum(V v) : value(v) {}
    std::string toString(){
        std::stringstream s;
        s << "value= " << value;
        return s.str();
    }
};

which would be used like this:

Extremum<int,std::greater,int > e(0,0);
e(10,3);
e(123,5);
std::cout << e.toString() << std::endl;

Extremum<int> g(0);
g(123);
g(1234);
g(1);
std::cout << g.toString() << std::endl;

I used a ´toString()´ because it is for debugging and printing to the screen while I might want to overload in/output operators for real in/output.

I dont like so much the ordering of the template parameters, as both the second and the third parameter would make a good fit as the last parameter so it can be ommited on the instantiation. However, I guess there is no better solution than simply choosing one ordering.

I have no idea how to provide good initial values. My idea was to use something like std::numeric_limits<V>::max, however, it depends too much on the choosen relation whether it should be max, min or something completely different. Maybe the easiest is let the user choose them when calling the constructor.

Aside from its functionaliy, which probably could be achieved with less code and a simple lambda or the like, is there anything inherently broken with this code? Anything obvious that could be improved?

Note that this is pre-C++11, but I would also be interested in any post-C++11 stuff.

Answer 1

Apart from cosmetic details, I see two things that could be improved IMHO.

The template comparator R<V>

As a template argument, you are asking for template <class> class R which is then used so R<V> r;

This narrows the possibilities since you can't pass a non-template comparator. For instance, if you have a custom type MyType and a custom comparator for that type MyTypeComp then you won't be able to use it with your Extremum struct because MyTypeComp doesn't accept template arguments.

The fix is easy:

template <
          typename V,
          class R,
          typename I = void
         >
struct Extremum {
    typedef V    value_type;
    typedef I    index_type;
    typedef R relation_type;
    V value;
    I index;
    R r;
    //...

And you would use it so:

Extremum<int, std::greater<int>, int> e(0,0);
Extremum<MyType, MyTypeComp, int> e(MyZero,0);

The index

You write almost the same code twice: once with index, once without. You could avoid it by passing the index in a pair. So you have only one class Extremum<V, R> and you set V = std::pair<Value, Index> and an appropriate comparator that compares two pairs according to their Value entry only.

template <class V, class R>
struct Extremum {
    typedef V    value_type;
    typedef R    relation_type;
    V value;
    R r;
    bool operator() (V v){
        bool result = r(v,value);
        if (result) {
            value = v; //you can use std::move here
        }
        return result;
    }
    Extremum(V v, R r) : value{v}, r{r} {} //you can use std::move here too
    std::string toString() const {
        std::stringstream s;
        s << "value= " << value;
        return s.str();
    }
};

template <class RV>
struct CompByFirstEntry {
    RV delegate;
    CompByFirstEntry(RV delegate);

    template <class V, class I>
    bool operator() (std::pair<V,I> const& lhs, std::pair<V,I> const& rhs) const {
        return delegate(lhs.first, rhs.first);
    }

    //or, even:

    template <class Pair>
    bool operator() (Pair const& lhs, Pair const& rhs) const {
        return delegate(lhs.first, rhs.first);
    }
} 

---

EDIT : Here is a working example.

• What changed: define the constructor CompByFirstEntry::CompByFirstEntry and operator<< for std::pair; choose one version of the operator().
• Note: you can also create a class ExtremumIndexed<V, I, R> that extends Extremum<std::pair<V,I>, MyPairComp<R>> or simply a template using directive.

---

std::ostream& operator<<(std::ostream& out, std::pair<int, int> const& p)
{
    out << p.first << " " << p.second;
    return out;
}

template <class V, class R>
struct Extremum {
    typedef V    value_type;
    typedef R    relation_type;
    V value;
    R r;
    bool operator() (V v){
        bool result = r(v,value);
        if (result) {
            value = v; //you can use std::move here
        }
        return result;
    }
    Extremum(V v, R r) : value{v}, r{r} {} //you can use std::move here too
    std::string toString() const {
        std::stringstream s;
        s << "value = " << value;
        return s.str();
    }
};

template <class RV>
struct CompByFirstEntry {
    RV delegate;
    CompByFirstEntry(RV delegate) : delegate{delegate} {}

    template <class V, class I>
    bool operator() (std::pair<V,I> const& lhs, std::pair<V,I> const& rhs) const {
        return delegate(lhs.first, rhs.first);
    }
};

int main(int argc, const char * argv[]) {
    using MyPairComp = CompByFirstEntry<std::greater<int>>;
    MyPairComp comp(std::greater<int>{});
    int value = 1;
    int index = 0;

    Extremum<std::pair<int,int>, MyPairComp> e({value, index}, comp);
    std::cout << e.toString() << std::endl;

    e({value + 1, index - 1});
    std::cout << e.toString() << std::endl;

    return 0;
}