views:

422

answers:

4

Starting from Visual Studio 2010, iterating over a set seems to return an iterator that dereferences the data as 'const data' instead of non-const.

The following code is an example of something that does compile on Visual Studio 2005, but not on 2010 (this is an artificial example, but clearly illustrates the problem we found on our own code).

In this example, I have a class that stores a position together with a temperature. I define comparison operators (not all them, just enough to illustrate the problem) that only use the position, not the temperature. The point is that for me two instances are identical if the position is identical; I don't care about the temperature.

#include <set>

class DataPoint
   {
   public:
      DataPoint (int x, int y) : m_x(x), m_y(y), m_temperature(0) {}
      void setTemperature(double t) {m_temperature = t;}
      bool operator<(const DataPoint& rhs) const
         {
         if (m_x==rhs.m_x) return m_y<rhs.m_y;
         else              return m_x<rhs.m_x;
         }
      bool operator==(const DataPoint& rhs) const
         {
         if (m_x!=rhs.m_x) return false;
         if (m_y!=rhs.m_y) return false;
         return true;
         }
   private:
      int m_x;
      int m_y;
      double m_temperature;
   };

typedef std::set<DataPoint> DataPointCollection;

void main(void)
{
DataPointCollection points;

points.insert (DataPoint(1,1));
points.insert (DataPoint(1,1));
points.insert (DataPoint(1,2));
points.insert (DataPoint(1,3));
points.insert (DataPoint(1,1));

for (DataPointCollection::iterator it=points.begin();it!=points.end();++it)
   {
   DataPoint &point = *it;
   point.setTemperature(10);
   }
}

In the main routine I have a set to which I add some points. To check the correctness of the comparison operator, I add data points with the same position multiple times. When writing the contents of the set, I can clearly see there are only 3 points in the set.

The for-loop loops over the set, and sets the temperature. Logically this is allowed, since the temperature is not used in the comparison operators.

This code compiles correctly in Visual Studio 2005, but gives compilation errors in Visual Studio 2010 on the following line (in the for-loop):

   DataPoint &point = *it;

The error given is that it can't assign a "const DataPoint" to a [non-const] "DataPoint &".

It seems that you have no decent (= non-dirty) way of writing this code in VS2010 if you have a comparison operator that only compares parts of the data members.

Possible solutions are:

  • Adding a const-cast to the line where it gives an error
  • Making temperature mutable and making setTemperature a const method

But to me both solutions seem rather 'dirty'.

It looks like the C++ standards committee overlooked this situation. Or not?

What are clean solutions to solve this problem? Did some of you encounter this same problem and how did you solve it?

Patrick

+6  A: 

The iterator should give you a const reference (and that's what the Standard says it should do), because changing the thing referred to would destroy the validity of the set's underlying data structure - the set doesn't "know" that the field you are changing is not actually part of the key. The alternatives are to make changes by removing and re-adding, or to use a std::map instead.

anon
If found this problem in code written by an ex-colleague. Personally I would use indeed use std::map, but to me it looks like custom comparison operators and set's don't fit well together in VS2010.
Patrick
Custom comparison and sets go along just fine, but mutable data-types and sets don't.
Space_C0wb0y
A: 

If you don't want to removing and re-adding as Neil suggests you could make setTemperature const and m_temperature mutable.

dalle
Just what I said in my question. Although this works, I still don't find this a decent solution.
Patrick
A: 

Set is supposed to return a const iterator because it doesn't know whether any member functions may change the ordering.

It seems like you really want a map, where you map your immutable (x, y) key to a variable temperature.

Alan
+1  A: 

Just recently began our conversion to 2010 and this was the biggest hurdle we've been facing. Fortunately they did reveal some long-standing problems where we were changing part of what made up the ordering of the set.

In other cases, our solution was to use mutable and declare methods as const. When passing the dereferenced iterator to a function by reference (either pointer or reference) then we made the argument consst if it wasn't being changed.

Dennis

Dennis