Martin Drozdik
Martin Drozdik

Reputation: 13313

How to elegantly avoid duplication of code when < changes to >?

Here is a simplified version of my code:

template<typename TIterator>
TIterator findMaximalPosition(TIterator begin, TIterator end)
{
    TIterator result(begin);
    for (TIterator it = begin + 1; it != end; ++it)
    {
        if ((*it)->value > (*result)->value) // Here I just need to change to "<"
            result = it;                     // to get a findMinimalPosition
    }
    return result;
}

template<typename TIterator>
TIterator findMinimalPosition(TIterator begin, TIterator end)
{
    // almost the same
}

This is just a simplified example. My code is full of places where two functions are the same except for a < or > sign or whether ++ or -- should be used.

My question is:

Is there a method how to reduce this duplication in code without

  1. Destroying the readability
  2. Decreasing the performance ?

I was thinking of using a pointer to an operator (either < or >) as a template parameter. This should not decrease performance, since the pointer would be a compile time constant. Is there some better or generally used way?

EDIT:

So what I did based on the answers was to implement:

template <typename TIterator, typename TComparison>
TIterator findExtremalPosition(TIterator begin, TIterator end, 
                               TComparison comparison);

and then just call:

return findExtremalPosition(begin, end, std::less<double>());

and

return findExtremalPosition(begin, end, std::greater<double>());

I hope this is what you meant. I suppose that after some struggling similar solution can be done for ++ and -- operators.

Upvotes: 1

Views: 109

Answers (2)

PaperBirdMaster
PaperBirdMaster

Reputation: 13298

As sugested by Ivaylo Strandjev, one possible solution is to use predicates.

So, if you change your function to work with predicates...

typename std::vector<int> vec;

template<typename TIterator, bool (*Predicate)(const TIterator &, const TIterator &)>
TIterator findPosition(TIterator begin, TIterator end)
{
    TIterator result(begin);
    for (TIterator it = begin + 1; it != end; ++it)
    {
        if (Predicate(it, result))
            result = it;
    }
    return result;
}

... and then, you define some predicates that helps you to achieve your goal...

bool lesser(const vec::iterator &a, const vec::iterator &b)
{
    return (*a) < (*b);
}

bool greater(const vec::iterator &a, const vec::iterator &b)
{
    return (*a) > (*b);
}

... then you would be able to do this:

vec::iterator min = findPosition<typename vec::iterator, lesser>(v.begin(), v.end());
vec::iterator max = findPosition<typename vec::iterator, greater>(v.begin(), v.end());

The advantage is to provide any function you would found useful, not only the ones to check if an element is greater or smaller than other:

bool weird(const vec::iterator &a, const vec::iterator &b)
{
    return ((*a) | (*b)) & 0x4;
}

vec::iterator weird = findPosition<typename vec::iterator, weird>(v.begin(), v.end());

Example here.

But before do this effort, check if the Algorithms library has already did the job.

I think that it looks pretty neat and simple.

Hope it helps.

Upvotes: 1

Ivaylo Strandjev
Ivaylo Strandjev

Reputation: 70989

I would make a general function that takes a predicate and use std::greater and std::less as argument to that function for the given type to implement findMaximalPosition and findMinimalPosition respectively.

Upvotes: 5

Related Questions