🎉 Celebrating 25 Years of GameDev.net! 🎉

Not many can claim 25 years on the Internet! Join us in celebrating this milestone. Learn more about our history, and thank you for being a part of our community!

Cannot insert into set. Please help.

Started by
7 comments, last by taby 4 years, 3 months ago

I'm having trouble. I cannot call the insert() member function of set.

The error is caused by line 218 of this Gist: https://gist.github.com/sjhalayka/3fe4333ed0588cca075a29b3b831ab3d

I don't think I'm doing anything special. I've tried all kinds of things, but nothing helps.

Thanks for any time you may have.

Advertisement

It would be nice if you included the error.

In any case, the problem is that the set you are trying to insert is part of the class, while the function you call on this class is const. const-member functions cannot modify member-variables. AddFileHFG should not be a const-function anyways logically.
Ignoring technical reasoning, only functions that do not modify the state of an object should be const (so pretty much only functions the get something, not set or add or delete).

Sorry about that. I'm not sure why I had that marked as const.

In any case, I took it out and now I'm facing the real problem, I cannot call a member function through an iterator. The line 219 is now the error-causing code: https://gist.github.com/sjhalayka/51e6910143616ae7006c62d66fc389ff

The error that I get is:

Error (active) E1086 the object has type qualifiers that are not compatible with the member function "history_resolution_group::AddFileHRG" wtf C:\dev\wtf\wtf\main.cpp 219

Ah, I see. Values inside a set cannot be modified (= are const), because any direct modification could cause a mismatch between the data and the preset structure for lookup.
In simpler words, think of an set<int>, internally it uses a tree so you can lookup the int. What if you would then assign another int to the iterator? The lookup would still happen as during insertion, but now looking up “3" could actually result in finding “5”.

You have one basic option:
Use a map instead of a set, and make the lookup by the maps key. map<uint64_t, history_resolution_group>. This seems like the natural solution to me, as making a lookup through a set the way you do it seems like it should really be a map. Using the object as the key appears hacky, sets are really more for seeing if something exists rather than actually doing stuff with the object inside that set.

Thanks again for pointing out the flaw to me. I tried several types of fixes, and the one I settled on was from stackexchange – history_resolution_group* elem = const_cast<history_resolution_group*>(&*i); // where i is an iterator

P.S. I'm redoing the app using Unreal Engine, and I will redesign the caching system in my app so that I won't have to use this hack that I found.

taby said:
Thanks again for pointing out the flaw to me. I tried several types of fixes, and the one I settled on was from stackexchange – history_resolution_group* elem = const_cast(&*i); // where i is an iterator

Const-casting is a terrible idea. Don't do it. I can safely say that at your level of experience, there is NEVER a valid reason for using a const-cast (in general there are only limited uses in advanced scenarios).

As I said, the right solution is replacing your set with a map. You are essentially misusing/abusing set here for what its not intended to.

history_resolution_group temp_hrg;

// => remove this parameter from history_resolution_group: temp_hrg.resolution;
// try to insert new hfg
// if exists already, will get iterator to existing, otherwise, will get iterator to new

pair< map<long unsigned int, history_resolution_group>::iterator, bool > pr;
pr = resolutions.insert(temp_params.resolution, temp_hrg);
pr.first->AddFileHRG(src_file_name, temp_params, src_creation_time);

This is just so that you understand it more in your code. You could also simplify it a bit further:

auto pr = resolutions[temp_params.resolution]; // no need to spell out the exact type here
pr.first->AddFileHRG(src_file_name, temp_params, src_creation_time);

Thats why a map is far superior here. []-access will eigther get the element or construct a new one, without you having to manually create a temporary (which is not needed if the element already exists.

Thanks yet again.

I know that it is not the optimal solution, but it came down to add a handful of pointers, or rewriting hundreds of lines of code. This old app is called Julia 4D 2. I am now making Julia 4D 3, and it won't be using sets in this antiquated way. Thanks again!

This topic is closed to new replies.

Advertisement