[RESOLVED] Stange problem in std::map

[SkyNetTo]

I'm trying to load a dictionary in memory and everything goes fine. No error, no warning, no memory leak, nothing. Just trying to see how many elements my map has and I dicovered that it is loading less words that the dictionary has. Here is my code.

UINT CPM2Dlg::LoadVocabolary(void)
{
	CStdioFile pFile;
	CString temp;
	
	if(pFile.Open("vocabolario.txt",CFile::modeRead | CFile::typeText))
	{
		while(pFile.ReadString(temp))
		{
			dataHandler.vocabolary.words.insert(dict_words::value_type(temp,DecodeString(temp)));
		}
		pFile.Close();
	}
	return 0;
}

CString & CPM2Dlg::DecodeString(CString & str)
{
	int i,length;
	CString buffer;

	length = str.GetLength();
	
	for(i = 0; i < length; i++)
	{
		if(str[i] == 'a' || str[i] == 'b' || str[i] == 'c' || str[i] == 'Ã*') 
		{
			buffer += "2";
			continue;
		}
		if(str[i] == 'd' || str[i] == 'e' || str[i] == 'f' || str[i] == 'è' || str[i] == 'é') 
		{
			buffer += "3";
			continue;
		}
		if(str[i] == 'g' || str[i] == 'h' || str[i] == 'i' || str[i] == 'ì') 
		{
			buffer += "4";
			continue;
		}
		if(str[i] == 'j' || str[i] == 'k' || str[i] == 'l') 
		{
			buffer += "5";
			continue;
		}
		if(str[i] == 'm' || str[i] == 'n' || str[i] == 'o' || str[i] == 'ò') 
		{
			buffer += "6";
			continue;
		}
		if(str[i] == 'p' || str[i] == 'q' || str[i] == 'r' || str[i] == 's') 
		{
			buffer += "7";
			continue;
		}
		if(str[i] == 't' || str[i] == 'u' || str[i] == 'v' || str[i] == 'ù') 
		{
			buffer += "8";
			continue;
		}
		if(str[i] == 'w' || str[i] == 'x' || str[i] == 'y' || str[i] == 'z') 
		{
			buffer += "9";
			continue;
		}
	}
	str = buffer;
	return str;
}

the vocabolary has 245227 elements but the map only shows 222791. If I delete a word from the dictionary (delete fisically a line in the txt file) the number decrease (as I expected). Any clue? Have I done something wrong here?

[fransn]

Probably there are double (identical) entries in your input.

[SkyNetTo]

the dictionary is taken from the internet and as far as I know there are no duplicate elements.

[fransn]

Ok what happens if you use a multimap instead?

[SkyNetTo]

didn't try. I've to try and tell you.

[VictorN]

... as far as I know there are no duplicate elements.

How did you check/test the presence/absence the duplicate elements?

[SkyNetTo]

loaded 245227. Thanks. But why this?

[SkyNetTo]

The second value (the one wich I recover from DecodeString) is the same many times but the first one is always different.

[Philip Nicoletti]

In DecodeString, what is the purpose of setting "str=buffer" ?

1) Shouldn't you just return buffer ?
2) If you can just return buffer, then pass str by const reference.

[Philip Nicoletti]

And I forgot to add ... you would need to return buffer by value
not reference.

[Paul McKenzie]

The second value (the one wich I recover from DecodeString) is the same many times but the first one is always different.

1) You never gave us the definition of your map. That was the first thing you should have posted. The definition could make all of the difference in why you're not seeing all your entries.

2) Return by value, not reference.

3) That function could have been written a lot easier using a lookup table as opposed to all of those if() statements.

4) You could have easily tested if there were truly any duplicates by calling map::find() before the insertion into the map. If map::find() returns a valid iterator, then the claim that there were no duplicates would have been a false claim.

Regards,

Paul McKenzie

[Philip Nicoletti]

A little more info on why I suggested what I did ...

 dataHandler.vocabolary.words.insert(dict_words::value_type(temp,DecodeString(temp)));

value_type is pair<CString,CString> ... so basically you are executing:

pair<CString,CString>(temp,DecodeString(temp));

I think that you are assuming that the key (temp) will be the value
before the call to Decode(temp) ... this is not necessarily the case.
DecodeString(temp) changes the value of temp. If you print out
your map, my guess is that the key and value will be the same.

Here is a simplified version of your code (and replaced CString with
string). It prints out the size of the map is 1 (on my system) instead
of the expected 2.

#include <string>
#include <map>
#include <iostream>

using namespace std;


string & DecodeString(string & str)
{
    str = "1";

    return str;
}

int main()
{
    map<string,string> m;

    string temp1 = "n";

    m.insert(map<string,string>::value_type(temp1,DecodeString(temp1)));

    string temp2 = "m";

    m.insert(map<string,string>::value_type(temp2,DecodeString(temp2)));

    cout << m.size() << "\n";
}

[SkyNetTo]

Thank you for your all your suggestions. Let start clarifying some things.
My multimap is defines as:

typedef std::multimap<CString,CString>dict_words

I'm trying to delevelop some kind of T9 used in Cell Phones to write SMS and so I need the decoded string of all the words in the dictionary. When I insert the word in the map I'm expected that one cell will handle the value of the word and its decoded String wich is defines as:

Word: play; DecodedString: 7529 //don't know If I explaind myself.

Once the temp value is inseted in the map I don't need it any more, that's why I'm changing it's value with the content of the decodedString. You said I've to pass string by value and not by reference. You're right but the point is I haven't any clue how to do this (at the right way it is intended). If I place

return buffer;

the compiler says that you are passing a temporary value wich goes out of scope and that's why I copied the context of buffer into str. If there's a better way of doin' this please let me know.
Again thanks to all of you. I'm learning so many things on this forum that I wasn't able to learn anywhere else.

[Philip Nicoletti]

Thank you for your all your suggestions. Let start clarifying some things.
My multimap is defines as:

Once the temp value is inseted in the map I don't need it any more, that's why I'm changing it's value with the content of the decodedString.

As I mentioned above, I believe that the "temp" value gets changed
before it gets inserted as the key into the map.

return buffer;

the compiler says that you are passing a temporary value which goes out of scope and that's why I copied the context of buffer into str. If there's a better way of doin' this please let me know.

return by value instead. So the signature for DecodeString would be:

CString  CPM2Dlg::DecodeString(const CString & str)

(no & after the first CString)

[SkyNetTo]

Shame on me. As you said the value of temp was changing before inserting it to map. So my map has only numbers and not words. Everythins works now even if I change the typedef from multimap to map. That was the reason why I had less cells than the words of the dictionary.