4

I want a custom comparator for the following code. However, I am not allowed to overload operator(), std::less, std::greater.

I tried to achieve this using lambda but gcc won't allow me to use auto as a non-static member. Any other way to make this work?

#include <iostream>
#include <map>
#include <set>

class Test 
{
public:
    // bool operator () (const int lhs, const int rhs) { // not allowed
    //     return lhs > rhs;
    // };    
    using list = std::multiset<int  /*, Test*/>;
    std::map<const char*, list> scripts;
};

int main() 
{
    Test t;
    t.scripts["Linux"].insert(5);
    t.scripts["Linux"].insert(8);
    t.scripts["Linux"].insert(0);

    for (auto a : t.scripts["Linux"]) {
        std::cout << a << std::endl;
    }

    std::cout << "end";
}

Edit: With lambdas

class Test 
{
  public:
    auto compare = [] (const int a, const int b) { return a < b;}
    using list = std::multiset<int, compare>;    //here
    std::map<const char*, list> scripts;
};

Error:

'auto' not allowed in non-static class member
 auto compare = [] (const int a, const int b) { return a < b;}
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • 1
    *"However, I cannot overload operator()"* Do you mean it didn't work, or you're not allowed to do it? – HolyBlackCat May 31 '19 at 19:36
  • 4
    What types are you planning to compare? By the way, comparing the const char* values is not a good idea because this is just a comparison of the pointers; moreover, using the same string literal, i.e. "Linux" could actually produce two different pointers. I would recommend to use std::string as a key in std::map – Dmitry Kuzminov May 31 '19 at 19:36
  • @HolyBlackCat not allowed – Sumit Dhingra May 31 '19 at 20:02
  • @Dmitry Kuzminov ouh. You're right. I just threw a sample program together but thank you for pointing this out. – Sumit Dhingra May 31 '19 at 20:04
  • Where is your attempt to make a lambda work? Where is this "auto" that was not allowed? (Showing what you've tried so far is a good idea if you want a more useful answer.) – JaMiT May 31 '19 at 21:26
  • I've edited the question. @JaMiT – Sumit Dhingra Jun 01 '19 at 04:17

3 Answers3

7

I want a custom comparator for the following code. However, I cannot overload operator(), std::less, std::greater.

I assume that you are not allowed to overload operator() of the Test class, but could be that of other class. If so, create a internal private functor which overloads operator() and that could be part of the alias using list = std::multiset<int, Compare>;

class Test
{
private:
    struct Compare
    {
        bool operator()(const int lhs, const int rhs) const /* noexcept */ { return lhs > rhs; }
    };

public:
    using list = std::multiset<int, Compare>;
    std::map<std::string, list> scripts;
};

I tried to achieve these using lambdas but gcc won't allow me to use auto as a non-static member. Any other way to make this work?

Update: After researching a while, I found a way to go which does work with a lambda function.

The idea is to use the decltype of std::multiset with custom lambda compare as the key of the std::map scripts. In addition to that, provide a wrapper method for inserting the entries to the CustomMultiList.

Complete example code: (See live)

#include <iostream>
#include <string>
#include <map>
#include <set>

// provide a lambda compare
const auto compare = [](int lhs, int rhs) noexcept { return lhs > rhs; };

class Test
{
private:
    // make a std::multi set with custom compare function  
    std::multiset<int, decltype(compare)> dummy{ compare };
    using CustomMultiList = decltype(dummy); // use the type for values of the map 
public:
    std::map<std::string, CustomMultiList> scripts{};
    // warper method to insert the `std::multilist` entries to the corresponding keys
    void emplace(const std::string& key, const int listEntry)
    {
        scripts.try_emplace(key, compare).first->second.emplace(listEntry);
    }
    // getter function for custom `std::multilist`
    const CustomMultiList& getValueOf(const std::string& key) const noexcept
    {
        static CustomMultiList defaultEmptyList{ compare };
        const auto iter = scripts.find(key);
        return iter != scripts.cend() ? iter->second : defaultEmptyList;
    }
};


int main()
{
    Test t{};
    // 1: insert using using wrapper emplace method
    t.emplace(std::string{ "Linux" }, 5);
    t.emplace(std::string{ "Linux" }, 8);
    t.emplace(std::string{ "Linux" }, 0);


    for (const auto a : t.getValueOf(std::string{ "Linux" }))
    {
        std::cout << a << '\n';
    }
    // 2: insert the `CustomMultiList` directly using `std::map::emplace`
    std::multiset<int, decltype(compare)> valueSet{ compare };
    valueSet.insert(1);
    valueSet.insert(8);
    valueSet.insert(5);
    t.scripts.emplace(std::string{ "key2" }, valueSet);

    // 3: since C++20 : use with std::map::operator[]
    t.scripts["Linux"].insert(5);
    t.scripts["Linux"].insert(8);
    t.scripts["Linux"].insert(0);

    return 0;
}

Until lambda are not default constructable and copyable. But, the std::map::operator[] does requered the mapped_type to be copy constructible and default constructible. Hence the insertion to the value of the scripts map(i.e. to std::multiset<int, decltype(/*lambda compare*/)>) using subscription operator of std::map is only possible from from C++20.

JeJo
  • 30,635
  • 6
  • 49
  • 88
3

There is a problem with your approach even if you could define a lambda the way you want to. Take a look at the multiset declaration:

template<
    class Key,
    class Compare = std::less<Key>,
    class Allocator = std::allocator<Key>
> class multiset;

Notice how each template parameter is a type (using the class keyword). Now look at how you tried to define your list:

using list = std::multiset<int, compare>;
                            ^      ^
                          type   value

The first parameter is good, but the second is a mismatch. The Compare parameter needs to be a type, not an object. One general way to resolve this sort of situation is to replace compare with decltype(compare), but that seems to be not what you want (plus it is problematic for lambda types). You seem to want a default constructed list to use compare instead of merely a default constructed object of the same type.

So what you need is a class whose default constructed object implements operator() in a way that gives the order you want. Since we're dealing with int, the standard library has some ready-made types for this purpose, namely std::less and std::greater.

using list = std::multiset<int, std::greater<int>>;

However, I cannot overload operator(), std::less, std::greater.

Hmm... this suggests that the example code may have been over-simplified, as an overload is not called for. OK, let's suppose the list is of some type that's harder to deal with, say:

class I { /* Internals not important for this example. */ };
using list = std::multiset<I, ???>;

If you are allowed to modify I, then the simplest approach might be to define operator> (or operator<) for objects of type I. Since std::greater (or std::less) uses this operator, you get the desired order from the standard template without overloading it.

If you are not allowed to modify I, then I think you're left with writing your own function object, as this is one situation where a lambda is inadequate. Fortunately, classes implementing function objects are easy to write; lambdas have superseded them in other situations mostly because the lambda syntax tends to be more convenient.

struct CompareI {
    bool operator() (const I & lhs, const I & rhs) const { return /* fill this in */; }
};
using list = std::multiset<I, CompareI>;

While this defines an operator(), it is not an overload. So it should satisfy the requirements given to you.

JaMiT
  • 14,422
  • 4
  • 15
  • 31
2

You can use a function pointer of a comparison function in the constructor:

main.cpp

#include <iostream>
#include <set>

using compType=bool(*)(int lhs, int rhs);

bool custom_compare_function(int lhs, int rhs)
{
    return lhs>rhs;
}


using list = std::multiset<int,compType>;
int main() {
    list l(&custom_compare_function);
    l.insert(1);
    l.insert(4);
    l.insert(2);
    for (auto& item: l) std::cout<<item<<std::endl;
}

produces the output

$ g++ main.cpp 
$ ./a.out 
4
2
1
phinz
  • 1,225
  • 10
  • 21
  • A quick question! How will I initialize the list comparator function in this case `std::map`? Because map will default construct if key is not present and it will initialize the comparator to null probably – Sumit Dhingra May 31 '19 at 20:10
  • @SumitDhingra Sorry you can't you can only write comparators for the first argument of map, that is the point. Please write a proper question to get any more detailed answer. – Gem Taylor May 31 '19 at 21:24
  • @GemTaylor @phinz This line in answer `list l(&custom_compare_function);` initializes the multiset comparator. Correct? But if I have something like `std::map` then how do I initialize the comparator of multiset? – Sumit Dhingra Jun 01 '19 at 04:22
  • You are right, in this case the map would not default construct. Assuming `map` is your map variable, you can check for existing keys by comparing `map.find()` against `map.end()`. If the key does not exist you can use `map.emplace(...)` to construct a new entry for the map with custom arguments for the constructor, see http://www.cplusplus.com/reference/map/map/ – phinz Jun 01 '19 at 05:34