How to iterate over a std::map full of strings in C++
C++DictionaryIteratorStdStdmapC++ Problem Overview
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: "error: no match for call to ‘(std::basic_string<char,
std::char_traits<char>, std::allocator<char> >) ()’".
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?
C++ Solutions
Solution 1 - C++
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
:
...append(iter->first) rather than ...append(iter->first())
As a matter of 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;
}
edit: facildelembrar pointed out (in the comments) that in modern C++ you can now rewrite the loop
for (auto& item: table) {
...
}
Solution 2 - C++
-
Don't write a
toString()
method. This is not Java. Implement the stream operator for your class. -
Prefer using the standard algorithms over writing your own loop. In this situation,
std::for_each()
provides a nice interface to what you want to do. -
If you must use a loop, but don't intend to change the data, prefer
const_iterator
overiterator
. That way, if you accidently try and change the values, the compiler will warn you.
Then:
std::ostream& operator<<(std::ostream& str,something const& data)
{
data.print(str)
return str;
}
void something::print(std::ostream& str) const
{
std::for_each(table.begin(),table.end(),PrintData(str));
}
Then when you want to print it, just 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 << data.first << "=" << data.second << "\n";
}
private: std::ostream& m_str;
};
DataMap table;
public:
void something::print(std::ostream& str);
};
Solution 3 - C++
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.
Solution 4 - C++
iter->first
and iter->second
are variables, you are attempting to call them as methods.
Solution 5 - C++
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
.
Solution 6 - C++
Use:
std::map<std::string, std::string>::const_iterator
instead:
std::map<std::string, std::string>::iterator
Solution 7 - C++
In c++11 you can use:
for ( auto iter : table ) {
key=iter->first;
value=iter->second;
}
Solution 8 - C++
Another worthy optimization is the c_str ( ) member of the STL string classes, which returns an immutable null terminated string that can be passed around as a LPCTSTR, e. g., to a custom function that expects a LPCTSTR. Although I haven't traced through the destructor to confirm it, I suspect that the string class looks after the memory in which it creates the copy.