2

I have a list with objects Studenti which are defined with multiple parameters. I want to sort this list of Students, first by their mean and if means are the same I sort them alphabetically.

In my class Student I declared this header function: bool mediaDescrescator(const&, const&); Which is implemented in this way:

bool Studenti::mediaDescrescator(const Studenti& a, const Studenti& b)
{
    if(a.medie_ != b.medie_)
    {
        return (a.medie_ > b.medie_);
    }

    return (a.nume_ > b.nume_);
}

medie_ is a double private member of Studenti nume_ is a std::string private member of Studenti

In main I have a list of Studenti: std::list<Studenti> listaStud_ = {stud1, stud2, stud3, stud4, stud5};

Function call: std::sort(listaStud_.begin(), listaStud_.end(), mediaDescrescator);

My error is: mediaDescrescator was not declared in this scope.

I've seen other topics about this type of sort and they are declared like mine, I tried even with vector type instead of list. mediaDescrescator is called without (), because it must be passed as a function pointer or function object.

Wolf Marian
  • 147
  • 12

5 Answers5

6

mediaDescrescator is a member function of Studenti. It's defined withing the lexical scope of the class. So you can access Studenti::mediaDescrescator, but not mediaDescrescator. There is no function named mediaDescrescator in the global namespace.

Also, to ward off the next error you may face, be sure it is a static member function. You don't need a valid instance to call it, after all. It doesn't access any member variables of this.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
5
  1. std::tie returns a std::tuple of references.

  2. std::tuple defines comparison operators which perform correct lexicographical comparisons.

So all you need to do is turn Studenti into a tuple with tie - then compare the tuples.

bool Studenti::mediaDescrescator(const Studenti& a, const Studenti& b)
{
    return std::tie(a.medie_, a.nume_) > std::tie(b.medie_, b.nume_);
}

You may find it useful to provide a method called as_tuple() on classes that you write:

struct Studenti
{
   // rest of class

   auto as_tuple() const { return std::tie(medie_, nume_); }
};
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
2

There are several problems with your implementation. The first, and the one that your compiler complains about is that mediaDescrescator is defined in the scope of Studenti and you try to access it in the global scope, use Studenti::mediaDescrescator to get a pointer to it. Second, mediaDescrescator is a normal member function of Studenti and as such it will need to be called on an object. This can be fixed, e.g. by making it a static member function instead, or by making it take one parameter and compare against the current object. Third, the std::sort function can not be applied to an std::list as it needs random-access iterators and std::list only provides bidirectional iterators.

Johan
  • 3,667
  • 6
  • 20
  • 25
  • You where right with the list. I've seen this in other topic. But I must use list type, how can I apply this type of sort without making a copy on a vector type? – Wolf Marian Nov 07 '17 at 14:15
  • 1
    https://stackoverflow.com/a/2432946/104774 says use `std::list<>::sort()` – stefaanv Nov 07 '17 at 14:33
1

Change function declaration to:

bool mediaDescrescator(const Studenti& a, const Studenti& b)

Basically have it outside the class.

If you don't want to make the members public, write getter functions for them and use them instead inside the sort.

Eyal K.
  • 1,042
  • 8
  • 23
  • It is a good practice to define functions outside the class in order to do stuff on class objects, even with get and set? – Wolf Marian Nov 07 '17 at 14:13
  • 1
    If you use the sort function only once, I would suggest writing a lambda function only in that place. Best practice is to have as much as possible self contained in the class, but sometimes it's not worth the time. – Eyal K. Nov 07 '17 at 14:44
1

Firstly, if mediaDescrescator() is a non-static member function, it only needs one "other" parameter because it can work the this member.

Secondly, lambdas are easy to have algorithms work with member functions (not tested):

listaStud_.sort([](const studenti& a, const studenti& b) { return a.mediaDescrescator(b); });

You could also implement the order function directly in the lambda as combined with Richard Hodges' answer (in this case with public members):

listaStud_.sort([](const studenti& a, const studenti& b) { return std::tie(a.medie_, a.nume_) > std::tie(b.medie_, b.nume_); });

Based on Johan's answer: std::sort can't be used on std::list, use std::list<>::sort() instead as in my code snippets.

stefaanv
  • 14,072
  • 2
  • 31
  • 53