2

I defined a class Movie. And that class has a

static set<Movie> movies;

I also have a static member function:

static Movie& find_by_title(string);

I'm returning a reference, because I want to modify the movie once I find it (I want to rate it). So, in the implementation I have

Movie& Movie::find_by_title(string title) {
  set<Movie>::iterator it;
  for (it = movies.begin(); it != movies.end(); ++it)
    if (it->title == title) return *it;
}

But it returns an error, because *it has a type of const Movie (and the function returns Movie&). But then how to get that element in a way I can change it? A pointer would also be ok, anything that would make it possible to change the element afterwards.

movies.find(*it) also returns an iterator to a const Movie, so it also doesn't work when I try to return *(movies.find(*it)).

Janko
  • 8,985
  • 7
  • 34
  • 51
  • You cannot legally get a non-const reference to an element of a `std::set<>`. – ildjarn Mar 05 '12 at 23:18
  • 1
    I hope you don't plan to change this element once it is stored in `std::set` – LihO Mar 05 '12 at 23:23
  • Yeah, I planned on changing it. I wanted that set to act like a database (just a temporary one, of course). I tried implementing the "database" with writing and reading from files on disk, but files are too complicated for me in C++, so I chose this approach. – Janko Mar 05 '12 at 23:29
  • 3
    Refer to [what happens when you modify an element of an std::set](http://stackoverflow.com/questions/908949/what-happens-when-you-modify-an-element-of-an-stdset). – Jesse Good Mar 05 '12 at 23:31
  • That cleared things for me, thanks. Post it as an answer so I can accept it. – Janko Mar 05 '12 at 23:36

2 Answers2

3

You can return a const reference, and make the methods for setting the rating const, and declare the member variables you have to change to be mutable... but that's probably not a good idea. mutable members are really intended for things that aren't logically part of the classes 'value'.

You're probably best off not using a std::set. Sets are a special purpose container that makes certain guarantees with pretty narrow uses. Here's an article that talks about what std::set is good for, and it outlines four conditions that should hold before you decide to use one:

  • The collection can potentially grow so large that the difference between O(N) and O(log N) is important.
  • The number of lookups is the same order of magnitude as the number of insertions; there aren't so few insertions that insertion speed is irrelevant.
  • Elements are inserted in random order, rather than being inserted in order.
  • Insertions and lookups are interleaved; we don't have distinct insertion and lookup phases.

When all of those conditions hold then std::set may be a good option. Otherwise, you may well be best off using a vector (probably wrapped in a class that provides a set-like interface).

bames53
  • 86,085
  • 15
  • 179
  • 244
  • 1
    I've written a class that wrapped a vector with a map's interface. Turned out the map was faster. Given that, I can't imagine _any_ reason one would want a vector wrapped with a set-like interface. +1 for everything else though. – Mooing Duck Mar 05 '12 at 23:52
  • @MooingDuck We've discussed the map vs. vector speed thing [before](http://stackoverflow.com/questions/9336239/why-is-vector-faster-than-map-in-one-test-but-not-the-other). The speed is quite dependent on several factors. It's not a given that the map is faster. I think it's reasonable to consider std::vector the 'default' container type and only use another container when there's a strong reason, such as the four conditions above holding, or perhaps if a different set of conditions holds: small input size and/or performance irrelevant, vector's interface fits badly but not worth adapting. – bames53 Mar 06 '12 at 00:20
3

The inner workings of std::set require that the element not be modified once it's inserted into the set. If you tried to modify it the internal structures would be inconsistent and bad things would happen.

If your data elements can be split into two parts, one that has a key which does not change and another which you will be changing, use a std::pair<const key,data> for the two parts and place them into a std::map.

Mark Ransom
  • 299,747
  • 42
  • 398
  • 622