How can I improve this design, which makes me declare a member function const and declare mutable variables?

Question

For some reason, I repeat the elements of the class in std::set and want to change the keys a bit, knowing that the order will not change.

The iterators on std::set are const_iterators , because if the key is changed, it can lead to the wrong order and, therefore, to damage the set. However, I know for sure that my operations will not change the order of my elements in the set.

At the moment, here is my solution:

 class Foo { public: Foo(int a, int b): a_(a),b_(b) {} ~Foo(){} bool operator < (const Foo& o) const { return this.a_ < o.a_ ; } void incrementB() const { ++b_; } // <-- the problem: it is not const! private: const int a_; mutable int b_; // <-- I would like to avoid this } void f() { std::set<Foo> s; // loop and insert many (distinct on a_) Foo elements; std::for_each(s.begin(), c.end(), [](const Foo& s) { s.incrementB(); }); // Foo must be const. iterators are const_iterators } 

How would you modify it (I know that I could use std::map , but I'm curious if you can suggest other options) to remove mutable and const?

thanks

Answer 1

You can not. Set elements must be const for the correct container:

This makes you realize that the key part must be unchanged, or the invariants of the data structure will be broken.

 struct element { std::string key_part; // const in the set bool operator<(const element&o) const { return key_part<o.key_part; } private: mutable int m_cached; // non-key, *NOT* used in operator< }; 

If you want to keep the ability to "express" a constant in the non-key part, divide it into pairs and save it on the map:

 std::map<std::string /*key_part*/, int /*m_cached*/> mapped; 

or, more flexibly:

 struct element { std::string key_part; // const in the set bool operator<(const element&o) const { return key_part<o.key_part; } struct value { int m_cached; int m_moredata; //... } /*not in the element itself*/; }; std::map<element, element::value> mapped; 

Answer 2

Another option is const_cast for the reference type:

 class Foo { public: void incrementB() const { ++ const_cast< int& >( b_ ); } private: int b_; }; 

But as already mentioned, you should not change the elements of the set.

Answer 3

One possibility might be to split the portion of the value of Foo in pimpl.

 class Element { public: Element(int key, int value); Element( const Element& el ); Element( Element&& el ); ~Element(); bool operator < (const Element& o) const; void incrementValue() const; int getValue() const; private: Element& operator=(const Element& ); Element& operator=( Element&& el ); struct Key { Key( const int key ) : m_KeyValue( key ) { }; const int m_KeyValue; }; struct Value; const Key m_Key; std::unique_ptr<Value> m_Value; }; struct Element::Value { Value( int val ) : value(val) { } int value; }; Element::Element(int key, int value) : m_Key(key), m_Value( new Element::Value(value) ) { } Element::~Element() { } Element::Element( const Element& el ) : m_Key( el.m_Key ), m_Value( new Element::Value( *el.m_Value ) ) { } Element::Element( Element&& el ) : m_Key(el.m_Key) { m_Value = std::move(el.m_Value); el.m_Value.release(); } bool Element::operator < (const Element& o) const { return m_Key.m_KeyValue < o.m_Key.m_KeyValue; } void Element::incrementValue() const { m_Value->value++; } int Element::getValue() const { return m_Value->value; } void f() { std::set<Element> s; s.insert(Element(1,2)); s.insert(Element(2,3)); std::for_each(s.begin(), s.end(), [](const Element& s) { s.incrementValue(); }); std::for_each(s.begin(), s.end(), [](const Element& s) { std::cout << s.getValue() << std::endl; }); } int main() { f(); return 0; } 

EDIT: Honestly, you have to decide if the extra level of indirection matters, or if you are better off using a map.