-2

I need to sort vector of structures by a given field.

To anyone who faces the same problem: I just made my own sort function and there you can pass in lambda as many variables as you want(not only two of them).


Here I used a lambda expression, but I can't define the field to sort by as lambda accepts only passed variables.

sort(patients.begin(), patients.end(), [](Patient a, Patient b) { return a.name > b.name; });

I can't do it just like this:

string sortfield = name;
    sort(patients.begin(), patients.end(), [](Patient a, Patient b) 
{ 
   if(sortfield == "name") return a.name < b.name;
   else if(sortfield == "age") return a.age < b.age;
...
});
  1. Because it will cause an error as lambda can use only passed variables, not local.
  2. It will be hard to expand. Each time when I will add new field to the structure, I will have to go back here and add one more "else if"

I also tried to use a pointer to a function-member, but it works the same way.

I could use switch-case like this:

enum Fields
{
    Name, Gender, DateOfBirth
};
Fields field;
switch (field)
{
case Fields::Name:
    sort(patients.begin(), patients.end(), [](Patient a, Patient b) { return a.name > b.name; });
    break;

case Fields::Gender:
    sort(patients.begin(), patients.end(), [](Patient a, Patient b) { return a.gender > b.gender; });
    break;

case Fields::DateOfBirth:
    sort(patients.begin(), patients.end(), [](Patient a, Patient b) { return a.dateOfBirth > b.dateOfBirth; });
    break;

default:
    break;
}

but then it will break OCP (Open Closed Principle). I will have to come back several times to add more "case" blocks when the structure will be expanded.

Is there a better way to do this?

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
Umar Bhat
  • 39
  • 3
  • "I also tried to use a pointer to a function-member, but it works the same way." its not clear what else you are looking for. You need to pick the field to be compared somehow. Maybe you can show the code where you use a member pointer and explain what is wrong with the code – 463035818_is_not_an_ai Apr 27 '22 at 07:47
  • 1
    To be honest I don't quite understand your problem, what are you trying to solve? – 康桓瑋 Apr 27 '22 at 07:48
  • To be honest, I'd go for the switch-case approach. Sometimes it's better not to overthink. Using std::tuple iso a struct might be the solution, but I'm not convinced the advantage of accessing the member based on input tops the disadvantage of working with tuples iso structs with named members. – stefaanv Apr 27 '22 at 08:10
  • Open Closed Principle. In future if the structure gets a new field, I would have to return to this code and add a new line of code, that is the problem of switch case @463035818_is_not_a_number – Umar Bhat Apr 27 '22 at 08:34
  • @stefaanv Thank you. You're right. Maybe i'll return to it later – Umar Bhat Apr 27 '22 at 08:37
  • if you use pointer to member you need no switch. – 463035818_is_not_an_ai Apr 27 '22 at 08:39
  • *"(as lamda can use only passed variables, not local."* -- umm... Heard of [captures](https://en.cppreference.com/w/cpp/language/lambda#Lambda_capture)? You just use the square brackets to introduce your lambdas and never put anything in them? – JaMiT Apr 28 '22 at 05:04
  • @JaMiT But the sort function isnt smart enogh to pass an additional variable in comparator(lambda). BUT you gave me an idea! I can just write my own sort function;) – Umar Bhat Apr 29 '22 at 12:47
  • "I just maked my own sort function" is never the right solution. – n. m. could be an AI Apr 30 '22 at 11:32
  • @UmarBhat *"But the sort function isnt smart enogh to pass an additional variable in comparator(lambda)."* -- I don't follow your logic. It seems like you are saying that you cannot capture a local variable in your lambda because the lambda must have exactly two parameters. But that's nonsense. It's like saying that you cannot paint the outside of a house red because the inside is blue. One of these does not restrict the other. – JaMiT Apr 30 '22 at 15:06

2 Answers2

2

You can use another lambda to pick the field to be compared:

template <typename F>
void my_sort(auto& patients, F f) {
    sort(patients.begin(), patients.end(), [f](const Patient& a,const Patient& b) { return f(a) > f(b); });
}


my_sort(patients,[](const Patient& a) { return a.name; });
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
1

Because it will cause an error as lambda can use only passed variables, not local.

This is wrong. You certainly can make use of local variables in a lambda. Capturing is what the square brackets are for.

string sortfield = name;
sort(patients.begin(), patients.end(), [&sortfield](Patient a, Patient b) 
{
    if(sortfield == "name") return a.name < b.name;
    else if(sortfield == "age") return a.age < b.age;
    // ...
});

On the other hand, this approach does add a maintenance burden if the Patient class is modified. You could keep your changes localized to the class implementation if you put the switching logic in the class. (Whether you use a string or an enumeration is up to you, but keep in mind that the compiler will not catch typos in a string.)

class Patient {
  public:
    enum Fields { Name, Gender, DateOfBirth };
    // For readability:
    using CompareFunction = std::function<bool(const Patient&, const Patient&)>;

    // This is a function that returns a functional. If you have not seen the
    // concept before, it might take some time to grasp the idea.
    static CompareFunction SortBy(Fields);

    // And the rest of the class definition
};

// Implementation:
Patient::CompareFunction Patient::SortBy(Fields field)
{
    switch (field)
    {
        // We are returning a lambda, which the sort function will invoke.
        case Name:        return [](const Patient &a, const Patient &b) { return a.name > b.name; };
        case Gender:      return [](const Patient &a, const Patient &b) { return a.gender > b.gender; };
        case DateOfBirth: return [](const Patient &a, const Patient &b) { return a.dateOfBirth > b.dateOfBirth; };
    }
    // ERROR! If we missed an enumerate, we should have the compiler complain.
    // Figure out what you want to do in this case.    
}

You will still have to update this if you want to add a new field for sorting (but not if you add a new member without the option to sort on it). However, this code is with the class implementation, so at least you do not have to hunt through your code base to find places to update.

To use this in your code that does the sorting:

Patient::Fields field = Patient::Name; // as an example
sort(patients.begin(), patients.end(), Patient::SortBy(field));

As a special case, if all the potential sort fields have the same type (not likely for patients, but possible for other situations) and public (not recommended in many scenarios), then a lambda with a pointer-to-member could be used instead of the SortBy method. The benefit of this approach is that there is no special maintenance when the class is modified. The limitation is that it is often not an option due to the restrictions (same type and public).

std::string Patient::* field = &Patient::name;
sort(patients.begin(), patients.end(), [field](const Patient &a, const Patient &b) {
    return a.*field < b.*field;
});

I suppose the "public" restriction can be gotten around if you use pointers to member functions. The same-type restriction might still be an obstacle, and the syntax is messier.

auto field = &Patient::get_name; // assuming get_name() is the getter.
sort(patients.begin(), patients.end(), [field](const Patient &a, const Patient &b) {
    return (a.*field)() < (b.*field)();
});

At this point, the "another lambda" approach in the other answer might be preferable (on style grounds).

JaMiT
  • 14,422
  • 4
  • 15
  • 31