std::map<>:: operator[] is non-const, so you can't call it from inside a const function. Use find() instead.
Printable View
std::map<>:: operator[] is non-const, so you can't call it from inside a const function. Use find() instead.
Note that you will have problems if the key does not exist.
One option is to make the map mutable, which would allow you to leave the other code the way it is. This only makes sense if you want to allow users to specify an invalid key and you'd rather return a new default constructed someobject than throw any kind of error. Making the map mutable unfortunately no longer allows the compiler to error out when you accidentally modify it, but if the behavior I described is what you want then that might be the best solution.
You could also just throw an exception on an invalid key or document the function to say that invalid keys cause undefined behavior. Those options make more sense if you don't expect the user to ever specify an invalid key.
Someone mentioned once on this forum that is totally okay to have that kind of functions and just require from the user to check is_key_in_map() before calling the get_object() function so I thought it would be okay to make it this way and always do the checking first.
>> Someone mentioned once on this forum that is totally okay to have that kind of functions and
>> just require from the user to check is_key_in_map() before calling the get_object() function so
>> I thought it would be okay to make it this way and always do the checking first.
Sure it's ok. As I said, in that case you could just throw an exception on an invalid key or document the function to say that invalid keys cause undefined behavior. You still should make a decision on how to handle when find() doesn't find the key. If you just dereference the result of find, then make sure you document that invalid keys are not allowed and undefined behavior will result.
Or you can define the behavior by checking the result of find() against end() and throwing an exception, although it sounds like you don't want to add that complexity. If not, then add an assert that verifies the result of find is not end().
BTW, I'm assuming you understood that if you don't make the map mutable (which you shouldn't because it doesn't fit your use case) then you have to use find().
I dont want to allow users to specify an invalid key (this shouldnt happen).
So the only way is to go with find() (as I understood).. Do you think it would be better in this case to check is the key exists and throw exception if it doesnt?
I think this is something similar to:
Should I check with all that kind of functions if the name exists (and throw exception in other case) although users shouldnt be allowed do get() before doing the check first.Code:someclass {
public:
std::string get_name(int n) { return m_names[n]; }
private:
std::vector<std::string> m_names;
};
You should check before the user does a get, I think.
Also note that operator [] inserts an element if it doesn't exist, so if the user did specify an invalid key, it would insert a new key instead of complaining. So yes, I think find and throwing an error might be a good approach.
Yes, it would.
std::map always inserts an object if it does not exist, when using the index operator.
If you only want to find and return an object, use find.
Find returns an iterator which you have to check against yourmap.end() (if it's equal, then the element doesn't exist) because the standard library was so poorly designed.
I think this is one area where the standard library was well designed instead.Quote:
Originally Posted by Elysia
EDIT:
On second thought, if you are referring to the need to compare with map.end(), then perhaps that could have been done differently, e.g., to return a singular iterator that evaluates to false, whereas all non-end iterators evaluate to true. On the other hand, the safe bool idiom was not known at the time these class templates were designed, conversion to void* had its problems, and comparison with end() is consistent with the usual range comparison, so it is still a stretch to say that "the standard library was so poorly designed" in this area.
>> Should I check with all that kind of functions if the name exists
Again, that's up to you. If you know the calling functions will be smart and not use an invalid key, then just assert in debug mode if find() returns end(). In Release mode you would just use the result of find directly and if the user uses a key that doesn't exist then it is their fault.
Throwing an exception is a good choice, but it has performance implications. If you don't have other checks in place in your code then using an exception is a good idea.
Next thing you'll be telling a rocket scientist how to get a space shuttle to the moon.
In short, you're not qualified to make such a judgement. nor am I, you could say
I see you've learnt the CRTP.
The code you posted beginning with the following, is an over-engineered solution looking for a problem:Code:template<typename Type, typename Class> class CUtilityMember
...
I knew I would get blamed for that :p
But that's OK, we all have our opinions.
But let's just say it isn't a judgement, it's an opinion. A judgement it is when everyone agrees.
Don't do it. Why do you want to return a reference to your private data? If your object is really a POD, make it a struct.
I'd also revisit the design. If you have a bunch of classes passing around and operating on a data struct, you may want encapsulate that data in an object with real behavior.