2

I am trying to create a map using STL C++. But I am getting into problems and am not able to figure out whats exactly is wrong.

For demonstration purposes I put down my code here.

#include <map>
#include <iostream>
using namespace::std;

struct COORD{

    int X;
    int Y;

    bool operator<(const COORD &other) const { return X == other.X && Y == other.Y;}

    COORD(int x, int y) {
        X = x;
        Y = y;
    }
 };

struct CAR{

    double speed;
    double model;
    double direc;
    CAR() {}
    CAR(double c1, double c2, double c3) {
            speed = c1;
            model = c2;
            direc = c3;
    }
};

int main(int argc, char **argv) {

    map <COORD, CAR> p;
    p.insert(pair<COORD, CAR>(COORD(20, 20), CAR(10.0, 10.0, 10.0)));
    p.insert(pair<COORD, CAR>(COORD(20, 30), CAR(20.0, 10.0, 10.0)));

    CAR c1 = p.at(COORD(20, 30));
    cout << c1.speed << "\n";

    return 0;
}

So when you execute the code, the new inserted value is not displayed. Actually if you try to update the old one that also doesn't work. Can anyone let me know whats wrong. Why this is happening?

Psypher
  • 396
  • 1
  • 3
  • 13
  • 1
    Your `operator <` looks somewhat suspect (you've implemented an operator == by the looks of it) – benjymous Oct 29 '13 at 13:06
  • 6
    You probably should not use class names like _ _ CAR _ _ for your classes. Double-underscore identifiers are reserved. – Wilbert Oct 29 '13 at 13:07
  • 3
    Plus they are horrible to read and type. Here's a reference: http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier – Retired Ninja Oct 29 '13 at 13:09
  • I would recommend using std::make_pair when inserting pairs in to a map – Ben J Oct 29 '13 at 13:51

1 Answers1

14

Your less-than comparison must implement a strict weak ordering. This is a requirement, without which, the maps cannot work. Your less-than operator does not respect that.

This is an example of a comparison that would work:

#include <tuple> // for std::tie

bool operator<(const COORD &other) const 
{
  return std::tie(X, Y) < std::tie(other.X, other.Y);
}

where the std::tie comparison is lexicographical, using X first, then Y. I also changed the name of the class to COORD because __COORD__ is a reserved name.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • The answer is just perfect. And thanks everyone for pointing out the correct method for writing identifiers. – Psypher Oct 29 '13 at 13:24
  • Just one more question. If i insert a new CAR at the same key. It doesn't update it. What could be the fault? – Psypher Oct 29 '13 at 13:47
  • 1
    @Psypher That is by design, see [here](http://en.cppreference.com/w/cpp/container/map/insert). If you want to over-write a value with an existing key, or insert a new element if one isn't present, use `operator[]`, i.e. `p[SomeKey] = SomeValue;`. – juanchopanza Oct 29 '13 at 13:51
  • but as you know the structure is complex type therefore I cannot really use it in `operator[]`. How can this be then accomplished. In your opinion am I approaching the problem in a hard way or there can be a simple (other method) all i want is to create a look up table so later I can use 'USER' and assign them a car based on `USER` coordinate i.e. based on `USER` coordinate look for the near by car and assign it to him – Psypher Oct 29 '13 at 14:13
  • 1
    @Psypher of course you can use it in `operator[]`, that is how maps work. I don't follow your user-coordinate thing, but it sounds like a different question. – juanchopanza Oct 29 '13 at 14:18
  • Yes you indeed correct. I found my mistake. I didn't use a default contructor in my CAR structure and therefore the compiler was complaining about not finding the right object. – Psypher Oct 29 '13 at 14:47