HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simpletemplatethanmorevaluetypefirstthatdoesstoring

Problem

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 
#include 
#include 

template  class R = std::greater,typename I=void>
struct Extremum {
    typedef V    value_type;
    typedef I    index_type;
    typedef R relation_type;
    V value;
    I index;
    R 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  class R>
struct Extremum {
    typedef V    value_type;
    typedef void index_type;
    typedef R realtion_type;
    V value;
    R 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 e(0,0);
e(10,3);
e(123,5);
std::cout  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::max, however, it depends too much on the choosen relation whether it should be max, `m

Solution

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

The template comparator R

As a template argument, you are asking for template class R
which is then used so R 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 
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> e(0,0);
Extremum 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 and you set V = std::pair and an appropriate comparator that compares two pairs according to their Value entry only.

template 
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 
struct CompByFirstEntry {
    RV delegate;
    CompByFirstEntry(RV delegate);

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

    //or, even:

    template 
    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



  • Note: you can also create a class ExtremumIndexed that extends Extremum, MyPairComp>` or simply a template using directive.



std::ostream& operator const& p)
{
    out 
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 
struct CompByFirstEntry {
    RV delegate;
    CompByFirstEntry(RV delegate) : delegate{delegate} {}

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

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

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

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

    return 0;
}

Code Snippets

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;
    //...
Extremum<int, std::greater<int>, int> e(0,0);
Extremum<MyType, MyTypeComp, int> e(MyZero,0);
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);
    }
}
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;
}

Context

StackExchange Code Review Q#133694, answer score: 4

Revisions (0)

No revisions yet.