C++ std::set custom comparator

Question

Well, my problem is that I'm using an std::set with a custom comparator, something like:

class A
{
public:
    A(int x, int y):
        _x(x), _y(y)
    {
    }

    int hashCode(){ return (_y << 16) | _x; }

private:
    short int _y;
    short int _x;
};

struct comp
{
    bool operator() (A* g1, A* g2) const
    {
        return g1->hashCode() < g2->hashCode();
    }
};

So, I use it like

std::set<A*, comp> myset;

// Insert some data
A* a = new A(2,1);
A* b = new A(1,3);
myset.insert(a);
myset.insert(b);

Now my problem is that I would like to do this:

myset.find( (2 << 16) | 1 );

But, of course, it excepts A* not short int.

So, I know I could use std::find_if, but won't it render useless the custom comparator? It would iterate the whole list, wouldn't it? Is there any way I could use find with the hashCode rather than the object itself?

Thank you!

How about std::find_if with the comparator (adjusted for equality)? — stardust, May 02 '13 at 12:03
Sorry that's what I meant when I wrote `std::find`, it should have been `std::find_if`. Wouldn't it iterate the whole list, and not optimize the search at all? My reason on using `std::set` is its O(log(n)) search cost. — , May 02 '13 at 12:05

score 2 · Accepted Answer · edited May 23 '17 at 10:24

2

set::find takes argument of type key_type (see discussion Why is set::find not a template?). Using a std::set you have to construct a temporary object to use find.

myset.find(A(2, 1));

If A is not cheap to construct you might want to use a std::map<int, A> (or a wrapper around it) instead.

edited May 23 '17 at 10:24

Community

1
1

answered May 02 '13 at 12:04

hansmaad

18,417
9
53
94

A is indeed not cheap. If I were to use a map, would it be better to use a `map` or an `unordered_map`. I'm currently using `google::dense_hash_map` for some other maps with int keys. Insert cost is not important at all, search cost is. – May 02 '13 at 12:09
@ProStage If ordering doesn't matter you should use `unordered_map` or `unordered_set`. – hansmaad May 02 '13 at 12:10
1

@JamesKanze That A is just an example not to write the +100 lines class code. "The thing is the class is not as simple. It actually holds one map which can grow up to 1000 elements. The number of A objects can also grow up to 1000+, all of them unique." (C&P from another comment). It's constructor is not, by any means, cheap. – May 02 '13 at 12:15
@hansmaad It does not matter in the sense I don't mind the order in which are stored, as long as that doesn't increase it's search time. I guess an hash table is ideal – May 02 '13 at 12:16

score 1 · Answer 2 · answered May 02 '13 at 12:11

1

You can't do this with std::set, because std::set<>::find is not a (member) template; the argument must be of the key type. For simple classes like yours, it's likely that using an std::vector<A> and keeping it sorted (using std::lower_bound for lookup, and as the insertion point) will be just as fast. And with std::lower_bound, you can pass in a comparitor, and use any type you want as key. All you have to do is ensure that your comp class can handle the mixed type comparisons, e.g.:

struct Comp
{
    bool operator()( A const&, B const& ) const;
    bool operator()( A const&, int ) const;
    bool operator()( int, A const& ) const;
};

answered May 02 '13 at 12:11

James Kanze

150,581
18
184
329

The thing is the class is not as simple. It actually holds one map which can grow up to 1000 elements. The number of `A` objects can also grow up to 1000+, all of them unique. Search time/cost is important, and vector wouldn't be an option. Also, threading issues would come when inserting/erasing, as all iterators get invalidated. – May 02 '13 at 12:13
@ProStage Then you'll have to find a work-around. A vector of `std::shared_ptr` might do the trick; alternatively, you might try to implement a "dummy" version of `A`, which isn't expensive to build, and which could be used as a key into a `std::set`. (A `map` with no entries isn't expensive to build. Or just move the implementation, or at least its expensive parts, into a delegate; an instance with a null pointer to the delegate can then be created as an index, and if the delegate is managed with a smart pointer, copy could end up very cheap.) – James Kanze May 02 '13 at 13:20

score 0 · Answer 3 · answered May 02 '13 at 12:11

0

myset.find(&A(2, 1));

Or

A a(2, 1);
myset.find(&a);

answered May 02 '13 at 12:11

Davit Samvelyan

198
8

1

The first is not legal C++. – James Kanze May 02 '13 at 12:11
It doesn't depend on the compiler. The standard clearly forbids it (unless class `A` has overridden `operator&`); is a compiler accepts it (when invoked as a C++ compiler), it is broken. (I'd actually be rather surprised if a compiler accepted it. This rule, after all, dates from the earliest days of C. But I've been surprised before at the things some compilers accept.) – James Kanze May 02 '13 at 15:35

score 0 · Answer 4 · answered May 02 '13 at 15:46

You have defined a std::set<A*, comp> myset;, so std::find() must take a A* argument.

std::set<A*, comp> myset;

// Insert some data
A* a = new A(2,1);
A* b = new A(1,3);
myset.insert(a);
myset.insert(b);

Then, you need to do

myset.find(&A(2,1))

Back to your question, std::find() is not taking your custom comparator. You need, in fact, to use std::find_if.

C++ std::set custom comparator

4 Answers4