8

I am using sets. I use a custom struct as the key. I am inserting a value and trying to find the inserted value. But it never seems to find the element.

I have overridden both the == operator and the < operator.

Here is the code of the structure:

struct distance_t
{
 public:
int id;
double distance;

bool operator<(const distance_t& rhs) const
{
    if(distance < rhs.distance)
        return true;
    else 
        return false;
}

bool operator==( const distance_t& rhs) 
{
    if(id == rhs.id)
        return true;
    else
        return false;
}
};

And this is the code of main

int main()
{
    set<distance_t> currentSet;

    distance_t insertDistance;
    insertDistance.id =1;
    insertDistance.distance = 0.5;

    currentSet.insert(insertDistance);

    distance_t findDistance;
    findDistance.id = 1;

    assert(currentSet.find(findDistance) != currentSet.end());
}

It always fails in the assert statement. What am I doing wrong?

Edit -Ok now I understand that it does not use the == operator at all. Here is what I want. I need the data structure to be ordered by distance. But I should be able to remove it using the id. Is there any clean way or already existing datastructure to do this?

mathematician1975
  • 21,161
  • 6
  • 59
  • 101
The Flying Dutchman
  • 582
  • 2
  • 7
  • 18
  • 2
    A style suggestion: where you have `if (expr) return true; else return false;` you could instead simply do `return expr;` – Blastfurnace Aug 23 '12 at 07:55
  • 1
    What's wrong with just `return id < rhs.id;` or `return id == rhs.id;`? (I'd consider using an `if` to return `true` or `false` an anti-pattern. The comparison operators return a `bool`, and testing a `bool` just to return it's value doesn't make sense.) – James Kanze Aug 23 '12 at 07:57
  • @Blastfurnace You don't need the parentheses, either. – James Kanze Aug 23 '12 at 07:58
  • @JamesKanze: True, I was trying not to transform his original code too much. – Blastfurnace Aug 23 '12 at 08:00
  • agreed. I had that first. But since it dint work, I tried different notations (even though it was a moot point.. worth a try!) – The Flying Dutchman Aug 23 '12 at 08:02
  • related : http://stackoverflow.com/questions/12069991/c-overloading-operators-difference-between-and/12070047#12070047 – Bartek Banachewicz Aug 23 '12 at 08:12
  • 2
    @TheFlyingDutchman Just trying random changes when something doesn't work isn't a good technique. If something doesn't work, don't change anything until you understand why it doesn't work. – James Kanze Aug 23 '12 at 08:50

3 Answers3

9

It fails because your less-than comparison uses distance_t::distance, which you are not setting in findDistance:

distance_t findDistance;
findDistance.id = 1;

std::set does not use operator== for anything. It only uses operator<. So you would have to change it's logic to use distance_t::id.

If you want to search by id without changing the set's ordering, you can use std::find:

set<distance_t>::iterator it = std::find(currentSet.begin(), 
                                         currentSet.end(), 
                                         findDistance);

This will use your operator==. Bear in mind that this has linear time complexity.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • I want to use distance to order the elements but want the id to find the element. Is that possible at all? – The Flying Dutchman Aug 23 '12 at 07:50
  • 1
    @TheFlyingDutchman ordering is used for look-up (set is a binary tree). So it is not possible to have decoupled ordering and lookup. You could incorporate `id` into the ordering logic, it really depends on what you are trying to achieve. – juanchopanza Aug 23 '12 at 07:51
  • 1
    @TheFlyingDutchman You can make `operator<` look at distance first, and if they're equal, look at id. That would preserve the order you're looking for. (And also change `operator==` to look at both fields.) But that does mean you cannot search if you only have the id. –  Aug 23 '12 at 07:53
  • 1
    @TheFlyingDutchman: `set::find` uses the set's ordering to perform a logarithmic lookup. You could iterate over the entire set looking for the desired `id` instead. – Blastfurnace Aug 23 '12 at 07:53
  • @TheFlyingDutchman you can search by `id` using `std::find`. I added an example. – juanchopanza Aug 23 '12 at 07:57
  • 1
    Put the objects into a `set` ordered by the distance, and maintain a map of `id`s to entries in the set. (If the types had identity, and were dynamically allocated, I'd suggest several `set `, with different ordering functions. But from the little you've shown, I don't think that this is the case.) – James Kanze Aug 23 '12 at 08:01
4

Because operator== is not invoked at all. Comparing elements is like:

!(a < b) && !(b < a)

In other words, it uses operator<.

Kiril Kirov
  • 37,467
  • 22
  • 115
  • 187
1

As you haven't assigned a value to findDistance.distance the result of the less then comparison is undefined.

Note that your definitions of the equality and less then comparison operators is dangerous, because it is easy to define instances of distance_t where their result is inconsistent. One example is two instances with the same distance but different id's.

Nicola Musatti
  • 17,834
  • 2
  • 46
  • 55