How to iterate over a STL map full of strings in C++

Question

I have the following issue related to iterating over an associative array of strings defined using std::map.

-- snip --
class something 
{
//...
   private:
      std::map<std::string, std::string> table;
//...
}

In the constructor I populate table with pairs of string keys associated to string data. Somewhere else I have a method toString that returns a string object that contains all the keys and associated data contained in the table object(as key=data format).

std::string something::toString() 
{
        std::map<std::string, std::string>::iterator iter;
        std::string* strToReturn = new std::string("");

        for (iter = table.begin(); iter != table.end(); iter++) {
           strToReturn->append(iter->first());
           strToReturn->append('=');
           strToRetunr->append(iter->second());
           //....
        }
       //...
}

When I'm trying to compile I get the following

error: "error: no match for call to ‘(std::basic_string, std::allocator >) ()’".

Could somebody explain to me what is missing, what I'm doing wrong? I only found some discussion about a similar issue in the case of hash_map where the user has to define a hashing function to be able to use hash_map with std::string objects. Could be something similar also in my case?

Thank you!

Answer 1

iter->first and iter->second are variables, you are attempting to call them as methods.

Answer 2

Your main problem is that you are calling a method called first() in the iterator. What you are meant to do is use the property called first...

I.e

...append(iter->first) rather than ...append(iter->first())

As a matter of advice / style, you shouldn't be using new to create that string

std::string something::toString() 
{
        std::map<std::string, std::string>::iterator iter;
        std::string strToReturn; //This is no longer on the heap

        for (iter = table.begin(); iter != table.end(); iter++) {
           strToReturn.append(iter->first); //Not a method call
           strToReturn.append('=');
           strToReturn.append(iter->second);
           //....
           // Make sure you don't modify table here or the iterators will not work as you expect
        }
        //...
        return strToReturn;
}

Answer 3

Change your append calls to say

...append(iter->first)

and

... append(iter->second)

Additionally, the line

std::string* strToReturn = new std::string("");

allocates a string on the heap. If you intend to actually return a pointer to this dynamically allocated string, the return should be changed to std::string*.

Alternatively, if you don't want to worry about managing that object on the heap, change the local declaration to

std::string strToReturn("");

and change the 'append' calls to use reference syntax...

strToReturn.append(...)

instead of

strToReturn->append(...)

Be aware that this will construct the string on the stack, then copy it into the return variable. This has performance implications.

Answer 4

Note that the result of dereferencing an std::map::iterator is an std::pair. The values of first and second are not functions, they are variables.

Change:

iter->first()

to

iter->first

Ditto with iter->second.

Answer 5

First off don't write a toString() method. This is not Java.
Implemt the stream operator for your class.

Secondly prefer to use the standard algorithms then writting your own loop.
In this situation std::for_each() provides a nice interface to what you want to do.

Thirdly if you must use a loop but don't intent to change the data
prefer const_iterator over iterator. That way if you accidently try and change the values the compiler will give you a little warning.

std::ostream& operator<<(std::ostream& str,something const& data)
{
    data.print(str);
    return str;
}
void something::print(std::ostream& str)
{
    std::for_each(table.begin(),table.end(),PrintData(str));
}

Then when you want to print it all you have to do is stream the object:

int main()
{
    something    bob;
    std::cout << bob;
}

If you actually need a string representation of the object you can then use lexical_cast.

int main()
{
    something    bob;

    std::string  rope = boost::lexical_cast<std::string>(bob);
}

The details that need to be filled in.

class somthing
{
    typedef std::map<std::string,std::string>    DataMap;
    struct PrintData
    {
         PrintData(std::ostream& str): m_str(str) {}
         void operator()(DataMap::value_type const& data) const
         {
             m_str << value.first << "=" << value.second << "\n";
         }
         private:  std::ostream& m_str;
    };
    DataMap    table;
    public:
        void something::print(std::ostream& str);
};