0

I have a function in a sorting header for mergeSort

This is my code:

template

void mergeSort(vector<Comparable> & a, vector<Comparable> & tmpArray, int left int right, Comparator cmp)
{
    if (cmp(left,right))
    {
        int center = (left + right) / 2;
        mergeSort(a, tmpArray, left, center);
        mergeSort(a, tmpArray, center + 1, right);
        merge(a, tmpArray, left, center + 1, right);
    }
}

I want to use this comparator and pass in the parameters in my mainDriver.cpp

class CompareXCoordinate {
public:
    bool operator()(const Point & p1, const Point & p2) const
    {
        return (p1.getX() < p2.getX());
    }
};

Currently I am passing it like this:

Points is vector of Point objects tempArr is an empty vector

mergeSort(points, tempArr, points.begin(), points.end(), Point::CompareXCoordinate::operator())

I get an error of C3867: non-standard syntax, use & to create pointer to member

Is that the proper way to pass a comparator or is it a different syntax?

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
Amor Diaz
  • 41
  • 5
  • 1
    [See the implemenation of merge sort here](https://stackoverflow.com/questions/24650626/how-to-implement-classic-sorting-algorithms-in-modern-c). 1) Better to use iterators, not full blown container types such as `vector` -- this makes it much more flexible. 2) Make the comparison a template argument and pass an instance of the comparison object (if necessary). – PaulMcKenzie Mar 09 '19 at 01:21
  • Pass an instance of the `CompareXCoordinate` class. – πάντα ῥεῖ Mar 09 '19 at 01:22

1 Answers1

1

operator() is an instance method, so you need to pass an instance of the CompareXCoordinate class as the comparator, not the operator() itself:

mergeSort(points, tempArr, points.begin(), points.end(), Point::CompareXCoordinate());

Though, your class does not act on any non-static data members (that would make sense if you wanted the use of < or > to be configurable), so you could just use a standalone function instead of a class:

bool CompareXCoordinate(const Point & p1, const Point & p2)
{
    return (p1.getX() < p2.getX());
}

mergeSort(points, tempArr, points.begin(), points.end(), Point::CompareXCoordinate);

Or, if you are using C++11 or later, you can use a lambda instead:

mergeSort(points, tempArr, points.begin(), points.end(),
    [](const Point & p1, const Point & p2){
        return (p1.getX() < p2.getX());
    }
);
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • No need for `(…)` around the return expr. – Marcelo Cantos Mar 09 '19 at 01:36
  • @MarceloCantos *technically*, yes, but that is a matter of personal style. – Remy Lebeau Mar 09 '19 at 01:36
  • Sure, but why introduce text that takes more effort and adds nothing to readability? – Marcelo Cantos Mar 09 '19 at 02:22
  • @MarceloCantos it may not add readability for you, but it can for other people. If you don't want to use it, that is your choice. Other people can choose differently. – Remy Lebeau Mar 09 '19 at 05:29
  • It doesn't add readability for anyone. Return is a statement. Its expression will never appear in a context requiring precedence disambiguation. – Marcelo Cantos Mar 09 '19 at 23:20
  • @MarceloCantos "*It doesn't add readability for anyone*" - that is your *opinion*, it is not a *fact*. And I'm done arguing this, it is not relevant to the OP's issue, or my answer to it. – Remy Lebeau Mar 10 '19 at 07:08
  • There are plenty of arguments against. More typing, more parentheses to keep track of (yours has two at the end, more complex expressions will have more, and adding an extra pair around everything is just plain more work for both author and reader), looks like a function call, redundant. This isn't a matter of opinion. Extra parentheses add no value and are actually worse by any measure. – Marcelo Cantos Mar 10 '19 at 13:30