views:

134

answers:

3

Hi. That's my first question :)

I'm storing the configuration of my program in a Group->Key->Value form, like the old INIs. I'm storing the information in a pair of structures.

First one, I'm using a std::map with string+ptr for the groups info (the group name in the string key). The second std::map value is a pointer to the sencond structure, a std::list of std::maps, with the finish Key->Value pairs.

The Key->Value pairs structure is created dynamically, so the config structure is:

std::map< std::string , std::list< std::map<std::string,std::string> >* > lv1;

Well, I'm trying to implement two methods to check the existence of data in the internal config. The first one, check the existence of a group in the structure:

bool isConfigLv1(std::string);
bool ConfigManager::isConfigLv1(std::string s) {
    return !(lv1.find(s)==lv1.end());
}

The second method, is making me crazy... It check the existence for a key inside a group.

bool isConfigLv2(std::string,std::string);
bool ConfigManager::isConfigLv2(std::string s,std::string d) {
    if(!isConfigLv1(s))
        return false;
    std::map< std::string , std::list< std::map<std::string,std::string> >* >::iterator it;
    std::list< std::map<std::string,std::string> >* keyValue;
    std::list< std::map<std::string,std::string> >::iterator keyValueIt;
    it = lv1.find(s);
    keyValue = (*it).second;
    for ( keyValueIt = keyValue->begin() ; keyValueIt != keyValue->end() ; keyValueIt++ )
        if(!((*keyValueIt).second.find(d)==(*keyValueIt).second.end()))
            return true;
    return false;
}

I don't understand what is wrong. The compiler says:

ConfigManager.cpp||In member function ‘bool ConfigManager::isConfigLv2(std::string, std::string)’:|
ConfigManager.cpp|(line over return true)|error: ‘class std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<const std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >’ has no member named ‘second’|

But it has to have the second member, because it's a map iterator...

Any suggestion about what's happening?

Sorry for my English :P, and consider I'm doing it as a exercise, I know there are a lot of cool configuration managers.

+2  A: 

keyValueIt is not a map iterator, it is a list iterator. You can just do

if (keyValueIt->find(d) != keyValueIt->end())
Joel
It is not a list<string>::iterator, it is a list<map<...> >::iterator. It cannot find d.
Beta
`*keyValueIt` gives a `map` and you can use `map::find(key)` on it.
Georg Fritzsche
You're right, my mistake.
Beta
Ok. I'll take Joel way. Also, I can replace: `return !(lv1.find(s)==lv1.end());`for: `return lv1.find(s)!=lv1.end();`in the first check method.Thank you guys!
fjfnaranjo
A: 

If you just want a group/key/value structure you are over-complicating it, you have one more level in your data structure then needed.
The additional list isn't needed, a map of maps is sufficient:

// typedefs for readability:
typedef std::map<std::string, std::string> Entries;
typedef std::map<std::string, Entries> Groups;
// class member:
Groups m_groups;

bool ConfigManager::hasKey(const std::string& group, const std::string& key) 
{        
    Groups::const_iterator it = m_groups.find(group);
    if(it == m_groups.end())
        return false;

    const Entries& entries = it->second;
    return (entries.find(key) != entries.end());
}
Georg Fritzsche
Thanks. Now I'm doing something like that ;)
fjfnaranjo
+1  A: 

I think Joel is correct with

if (keyValueIt->find(d) != keyValueIt->end())

However I wanted to encourage you use some typedefs to try and simplify your code. Using typedefs can help when diagnosing problems like this (and if you're lucky your compiler will give you more meaningful error messages as a result.

For instance:

typedef std::map<std::string,std::string> KeyValueMap;
typedef std::list< KeyValueMap > ConfigurationList;
typedef std::map< std::string, ConfigurationList* > ConfigurationMap;

bool isConfigLv2(std::string,std::string);
bool ConfigManager::isConfigLv2(std::string s,std::string d) {
    if(!isConfigLv1(s))
        return false;

    ConfigurationMap::iterator it;
    ConfigurationList* keyValue;
    ConfigurationList::iterator keyValueIt;  // <- it's not a keyValue iterator, it's a ConfigList iterator!
    it = lv1.find(s);
    keyValue = (*it).second;
    for ( keyValueIt = keyValue->begin() ; keyValueIt != keyValue->end() ; keyValueIt++ )
        if(!((*keyValueIt).second.find(d)==(*keyValueIt).second.end()))
            return true;
    return false;
}

Simplifying the types makes it more obvious to me that keyValueIt is probably being misued (i.e. it's actually a list iterator, and not a KeyValueMap iterator and so the '.second' access is erroneous.)

HulkHolden
PS my typedef names are pretty rubbish. It's worth spending the time thinking up good, descriptive names :)
HulkHolden
Don't worry, the idea is right to. gf resumes it pretty good in the next comment.
fjfnaranjo