Getting a field from an STL map iterator

Go To StackoverFlow.com

2

I have a map container to store certain objects, together with their name and type:

typedef std::map<std::string, std::pair<ObjType, ObjBase*> > ObjContainer;

However, in many parts of the code, there are constructions like this:

ObjContainer::iterator it = mObjContainer.find(name);
if (it != mObjContainer.end()) {
    if (it->second.second) {
        it->second.second->setObj2Default();
        delete it->second.second;
        it->second.second = 0;
    }
}

Obviously, the many "it->second.second" are not very clear, and unmaintainable. If it is changed in the future, to support one more field, for example, it will be all broken. So, I am trying to change them by functions to access the fields, like this:

ObjBase*& getObjPtr(ObjContainer::iterator it) {
    return it->second.second;
}

Similarly, also function getObjName and getObjType.

It was also suggested to me that it would be more clear to have the iterator returning those fields:

 it.objPtr();
 it.objName();
 it.objType();

But I think that the STL iterators should not be inherited to have those functions, right? I see no other way to do it except to create a wrapper for the map and have its own iterator with those functions.

So, what would be the most appropriate option? Is there any other way to solve this problem that I am not seeing?

2012-04-03 22:01
by Gustavo N
Why don't you just use a struct instead of a pair and name your fields they way you want - Spidey 2012-04-03 22:06
@Spidey: Why roll your own class definitions if the library already comes with a perfectly serviceable one - Kerrek SB 2012-04-03 22:13
You could wrap'em with acessors. At least you'll get away with not using pair.first, pair.second, pair->second->second, etc - Spidey 2012-04-03 23:01


4

If the biggest problem is maintainability, I would replace the std::pair with a custom class/struct that wraps the ObjType and ObjBase* as one.

  • it's easy to add a new field in the mix
  • it's easy to access struct fields ObjType and ObjPair
  • it's easy to write getters/setters/other functions for a class that handle ObjType and ObjPair
2012-04-03 22:10
by supertopi


2

I'd just make a local copy of the pointer (or reference) -- it'll probably be optimized out anyway:

ObjContainer::iterator const it = mObjContainer.find(name);
if (it != mObjContainer.end())
{
    ObjBase * & p = it->second.second;
    if (p) { p->foo(); delete p; p = NULL; }
}
2012-04-03 22:08
by Kerrek SB


0

Use a reference to simplify the syntax.

ObjContainer::iterator it = mObjContainer.find(name);
if (it != mObjContainer.end()) {
    std::pair<ObjType, ObjBase*> & ref = it->second;
    if (ref.second) { // ...
2012-04-03 22:09
by Mark Ransom


0

I'd first question myself on whether ObjType is mandatory there. If the aim is just to tell what kind of class is actually pointed by the second ObjBase* parameter of the pair, use dynamic_cast and get rid of the pair.

typedef std::map<std::string, ObjBase*> ObjContainer;

No more second.second in the code then:

ObjContainer::iterator it = mObjContainer.find(name);
if (it != mObjContainer.end()) {
    if (it->second) {
        it->second->setObj2Default();
        delete it->second;
        it->second = NULL;
    }
}

And when you need to test the actual type of the object:

ObjContainer::iterator it = mObjContainer.find(name);
if (it != mObjContainer.end()) {
    if (ChildObj* p_Child = dynamic_cast<ChildObj*>(it->second)) {
        // Work on p_Child...
    }
}
2012-04-03 22:35
by aroyer
No, the ObjType is needed, because sometimes the ObjBase* will be null, and they ObbType can be used to build a new instance using a factory - Gustavo N 2012-04-04 13:19