Thread: Finding a key in a map returns true always

Hello there,

I'm working on a simple modding tool for a game. A new version of the game just came out, and I am working on a tool to convert some of the old game's files into the new format. To give a little background on what I am trying to achieve, I'll tell you a little about the old format and the new format.

The old format was basically just a bunch of values separated by a semicolon. One line defined all the values for a particular minister. Each minister could hold multiple positions, but each new position counted him as a totally new minister, IIRC. Thus, a minister of, say, the army could die, but if he was also the chief of the navy, he would live on as leader of the navy.

In the new file format, they've changed it such that each minister has all of his possible positions defined at once. That's great from a gameplay perspective, but it makes the job of converting from one format to the other slightly more difficult. Instead of just writing everything out in the new format, I must now worry about whether or not a particular minister has already been defined. Because of the old file format, the only possible method of identifying the guy is by using his name, which comes as a simple string. The duplicates can be legally declared on any other line. They don't have to be on the next line, so for the moment, every name (except for the first one) has to be tested to see if it exists.

My approach was to use maps, where the key is a std::string and the data is stored in a minister struct. The key, since it is supposed to be unique, is the name of the minister. So, in theory, I should be able to use the map::find function in order to test to see if there is a minister who already has this name. So, let's take a look at how I've declared my map, how I'm feeding it data, and how I'm testing data.

Code:

	//declare my map
	map<string,minister> ministerList;
    .....
	//Feed the map some data
	//myMinister is a local variable of type minister
	ministerList[myMinister.name] = myMinister;
    .....
	//Test that the name is not taken
	//name is a string that the function is being passed
	bool test = false;
	map<string,minister>::iterator myIter;
	myIter = ministerList.find(name);
	test = myIter != ministerList.end();
	return test;

The theory is that, if the test is false, the name has not already been taken. Nothing special will need to be done on this go around. I want to be sure that my data is being read correctly before I actually write the part where it outputs to the new file. So I litter my code with cout statements, outputting the value of everything. I print out test for each line in the original file.

What I find is, the code reads the data fine. When it goes to test the first bit of data, no names have been put into the map, so it correctly returns false. The interesting thing is, it will always return true for all other passes, which is incorrect behavior. In the file I was using for test purposes, it should return true only for one line.

Any suggestions would be wonderful!

Originally Posted by dwks

Code:

myIter = ministerList.find(name);

What is name? Maybe the last name that you read from the file, which of course will always exist . . . ?

Well, that wasn't actually the case, but it turns out there was, indeed, a problem with the name variable. Where I had it declared caused it to go out of scope, making it default to 0 or NULL or something. Thus, every name ended up being NULL. *smacks forhead*

After changing the declaration, it now works as it should. Thanks for making me be suspicious of the name variable.

Originally Posted by dwks

[edit] By the way, it's perfectly valid to use

Code:

return myIter != ministerList.end();

[/edit]

I'm aware of that. It was late when I wrote that. My latenight mind, when trying to output that to the console, gave up when the compiler said that cout << test here; was not valid. It neglected to remember that I could have put parentheses around the test to make it output everything.

Well, thanks for the help.