sort std::set using operator() to order the insertions

Question

I am continuing this post after This we have a class as:

class LaneConnector {
public:

    const Lane* getLaneFrom() const {
        return From;
    }
    const Lane* getLaneTo() const {
        return To;
    }

private:

    Lane* From;
    Lane* To;
}

and a functor to compare as:

struct MyLaneConectorSorter {
  bool operator() (const LaneConnector* rhs, const LaneConnector* lhs) const
  {
    // you may want to put some null pointer checks in here
    const Lane* a = lhs->getLaneFrom();
    const Lane* b = rhs->getLaneFrom();
    return a->getLaneID() < b->getLaneID();
  }
};

and finally a source and destination set:

const std::set<LaneConnector*> src = ..... ;

const std::set<LaneConnector*, MyLaneConectorSorter> dest(src.begin(), src.end(), MyLaneConectorSorter());

The size of the dest set will be 1 while the src has more(14 in my case)

what might have I done wrong? I value your kind comments. Thank you

There was a typo in my answer to the original problem: can you try `dest(src.begin(), src.end(), MyLaneConectorSorter())`? I had missed the `()` after `src.begin`. — juanchopanza, Sep 25 '12 at 06:51
@ juanchopanza Hi, that is already added so the problem is not coming from there — rahman, Sep 25 '12 at 06:53
Do the LaneConnectors have different lane ids? Sets require unique ids, I'm not sure what happens in a set if the comparator says two keys are the same. — CrazyCasta, Sep 25 '12 at 07:01
Show real code! If the set dest is supposed to store pointers, your comparator also needs to take pointers! — sellibitze, Sep 25 '12 at 07:03
You seem to have some confusion in your comparison function with regards to right and left. It's also really bugging me that you spell `LaneConnector` with two 'n's and `MyLaneConectorSorter` with one. — BoBTFish, Sep 25 '12 at 07:03
@CrazyCasta: If you try to insert a new element in a set and it is already there (according to the comparer, of course), the insertion fails. It may well be what you say. — Gorpik, Sep 25 '12 at 07:04
@sellibitze the code is large. and yes the above snippt has a type. laneconnectors are passed by their pointers — rahman, Sep 25 '12 at 07:14
@Gorpik is there any way to find out if the insertion fails? — rahman, Sep 25 '12 at 07:15
@rahman: `insert` returns a `pair`, where the `bool` tells you whether the insertion worked or not. — Gorpik, Sep 25 '12 at 07:20
@Gorpik and I dont have insert. May be I should iterate through the old set and insert into new one one by one , though not very nice looking ? — rahman, Sep 25 '12 at 07:24
No, of course, but what the `set` constructor does is similar. Only it does not tell you which elements were added and which were not. You can use `insert` instead of the range constructor just to see where it is failing. — Gorpik, Sep 25 '12 at 07:29
@BoBTFish that is my fault. It was wrong in my answer to another question. I am left handed and often get hands mixed up. — juanchopanza, Sep 25 '12 at 07:31
The most likely scenario is that all your IDs are the same. Can't you print them out before making the second set? — juanchopanza, Sep 25 '12 at 07:32
@juanchopanza I am confused. Suppose the IDs are all same. I know the set elements are unique, but I thought the functor just stores them in order not discard them. Am I wrong? — rahman, Sep 25 '12 at 07:42
@rahman You are wrong. ``std::set`` has unique entries, and the uniqueness is determined by the comparison function. If your IDs are duplicates, the set considers the elements to be the same. — juanchopanza, Sep 25 '12 at 07:46
Maybe you want a `std::multiset` instead of a `std::set`. Anyway, I cannot understand what kind of order you are expecting from `MyLaneConnectionSorter` if all IDs are the same. — Gorpik, Sep 25 '12 at 07:48
@juanchopanza I got where I was going wrong. This is the duplicacy issue. So the main issue is solved, thanks to ALL of you, I will get back with some comments and will start another thread asking how to maintain a set with more than one key/ID for duplicacy checking. — rahman, Sep 25 '12 at 08:02

score 1 · Accepted Answer · answered Sep 25 '12 at 07:46

std::set keeps track of elements based on the key. In your comparator you have return a->getLaneID() < b->getLaneID();. Thus Lane ID implicitly becomes the key. Since if a and b have the same LaneID, then both MyLaneConectorSorter(a, b) and MyLaneConectorSorter(b, a) are returning false.

Your set thus can not contain more than one LaneConnectior with the same LaneID.

score 0 · Answer 2 · answered Sep 25 '12 at 07:22

There is a very simple way to catch problems like this way before they get a chance to expose themselves. Write unit tests!

My guess is that all your LaneConnectors start at the same line. So, GetLaneFrom()->GetLaneID() yields the same result on all LaneConnectors

sort std::set using operator() to order the insertions

2 Answers2

Linked