What Am I Doing Wrong?

[Mike Copeland]

The following program compiles and runs, but the code that detects
and processes an STL map error doesn't work. That is, although it
detects the problem and attempts to change the map key, the 2nd "find"
(in the else clause) doesn't get out of the main "while loop" where is
would store the new object. Please advise. TIA

#pragma warning (disable:4786)
#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <io.h>
#include <algorithm>
#include <string>
#include <map>
typedef char Str4[5];
using namespace std;
typedef struct TEAMMAPTYPE
{ // Team Ids, Names
Str4 teamCode; // Team Code
string teamName; // Team's Name
bool isAdded; // Added to working stack of teams
char teamTypeCode; // Team type Code
int teamMembers1; // Count of Team Members-1
int teamMembers2; // Count of Team Members-2
} teamData;
teamData teamWork; // storage for Team info
typedef map<char*, TEAMMAPTYPE> TEAMS;
TEAMS teamMap;
map<char*, TEAMMAPTYPE>::iterator teamIt;
int GTNumbs = 7000;

void addTeamInfo(Str4 tCode, string tName)
{
Str4 tWork;

strcpy(tWork, tCode);
teamIt = teamMap.find(tWork); // see if this Team exists
while(teamIt != teamMap.end()) // if key matches
{
teamWork = teamIt->second;
if(teamWork.teamName == tName) // same Code; same Name
{
return;
}
else // same Code; different Name
{
sprintf(tWork, "%4d", GTNumbs++); // change Code and
teamIt = teamMap.find(tWork); // see if this exists
}
} // while
teamWork.isAdded = true; // store new data object
teamWork.teamMembers1 = teamWork.teamMembers2 = 0;
strcpy(teamWork.teamCode, tWork);
teamWork.teamName = tName;
teamWork.teamTypeCode = '?';
teamMap.insert(TEAMS::value_type(tWork, teamWork));
return;
}

int main() // check if Team data exists; add it if not
{
addTeamInfo("1234", "My Team 1");
addTeamInfo("2345", "My Team 1");
addTeamInfo("1234", "My Team 2");
addTeamInfo("1234", "My Team 1");

size_t mSize = teamMap.size();

return EXIT_SUCCESS;
}

 

[Ian Collins]

On 11/ 6/10 12:52 PM, Mike Copeland wrote:

The sort answer to your topic is mixing C and C++ idioms!

The following program compiles and runs, but the code that detects
and processes an STL map error doesn't work. That is, although it
detects the problem and attempts to change the map key, the 2nd "find"
(in the else clause) doesn't get out of the main "while loop" where is
would store the new object. Please advise. TIA

typedef char Str4[5];
using namespace std;

typedef struct TEAMMAPTYPE

"typedef struct" is a C idiom. Using all caps of types is horrible.

{ // Team Ids, Names
Str4 teamCode; // Team Code

Use a real type or a std:;string.

string teamName; // Team's Name
bool isAdded; // Added to working stack of teams
char teamTypeCode; // Team type Code
int teamMembers1; // Count of Team Members-1
int teamMembers2; // Count of Team Members-2
} teamData;

Sensible names make descriptive comments redundant.

teamData teamWork; // storage for Team info

typedef map<char*, TEAMMAPTYPE> TEAMS;

char* is not a good choice for a map key unless you will only be using
unique string literals.

This will be the cause of your problems.

 

[Mike Copeland]

The short answer to your topic is mixing C and C++ idioms!

The following program compiles and runs, but the code that detects
and processes an STL map error doesn't work. That is, although it
detects the problem and attempts to change the map key, the 2nd "find"
(in the else clause) doesn't get out of the main "while loop" where is
would store the new object. Please advise. TIA

typedef char Str4[5];
using namespace std;

typedef struct TEAMMAPTYPE

"typedef struct" is a C idiom. Using all caps of types is horrible.

{ // Team Ids, Names
Str4 teamCode; // Team Code

Use a real type or a std:;string.

string teamName; // Team's Name
bool isAdded; // Added to working stack of teams
char teamTypeCode; // Team type Code
int teamMembers1; // Count of Team Members-1
int teamMembers2; // Count of Team Members-2
} teamData;

Sensible names make descriptive comments redundant.

teamData teamWork; // storage for Team info

typedef map<char*, TEAMMAPTYPE> TEAMS;

char* is not a good choice for a map key unless you will only be using
unique string literals.

This will be the cause of your problems.

I don't understand: it seems your comments are fundamentally _style_
issues, and it surprises me that such things would cause operational
failures. Are you suggesting that my use of char* is causing the second
map::find to fail, where the first one in the outer loop doesn't?
That's unlike anything I've ever seen in my 50+ years of programming.
Actually, I was thinking there might be some problem with the multiple
uses of the single iterator or the fact it isn't a "const" iterator
(aspects of C/C++ that I have little experience with).
I will adjust my code, but I truly don't understand why it would
work...if in fact it does. 8<{{

 

[Ian Collins]

That's more than just a comment on style!

I don't understand: it seems your comments are fundamentally _style_
issues, and it surprises me that such things would cause operational
failures. Are you suggesting that my use of char* is causing the second
map::find to fail, where the first one in the outer loop doesn't?

Yes, because you are altering the data the pointer points to, but you
are passing the same *pointer* to teamMap.find( tWork), so that
*pointer* will always be found!

That's unlike anything I've ever seen in my 50+ years of programming.

Maybe you've never searched for the same thing in an infantile loop
before...

Actually, I was thinking there might be some problem with the multiple
uses of the single iterator or the fact it isn't a "const" iterator
(aspects of C/C++ that I have little experience with).
I will adjust my code, but I truly don't understand why it would
work...if in fact it does. 8<{{

There's nothing wrong with reusing an iterator.

 

[Bo Persson]

Are you suggesting that my use of char* is
causing the second map::find to fail, where the first one in the
outer loop doesn't?

Yes, you are mapping the pointer (the address) and not the string it
points to. This might work for string literals, as they stay the same
for the lifetime of the program, but not for anything else.



Bo Persson

 

[Mike Copeland]

char* is not a good choice for a map key unless you will only be using

Yes, because you are altering the data the pointer points to, but you
are passing the same *pointer* to teamMap.find( tWork), so that
*pointer* will always be found!


Maybe you've never searched for the same thing in an infantile loop
before...


There's nothing wrong with reusing an iterator.

Well, I changed my code to conform to your comments...and it now
works. (!) I am surprised, as I said, because I didn't grasp the
nuances of the data and pointers I had invoked. I suppose this falls
into the arena of Undefined Behavior, which (now) makes sense. <sigh>
Thanks for your help (and patience). Perhaps I learned something
here... 8<}}

 

[Ian Collins]

On 11/ 7/10 04:02 AM, Mike Copeland wrote:

Please don't snip attributions!

Well, I changed my code to conform to your comments...and it now
works. (!) I am surprised, as I said, because I didn't grasp the
nuances of the data and pointers I had invoked. I suppose this falls
into the arena of Undefined Behavior, which (now) makes sense.<sigh>

No, there wasn't any UB, just a misunderstanding! Just remember that
the key for a map is a value and that pointer values are compared as is
and not dereferenced.

Thanks for your help (and patience). Perhaps I learned something
here... 8<}}

All part of the service

 

[James Kanze]

Mike Copeland wrote:

Yes, you are mapping the pointer (the address) and not the
string it points to. This might work for string literals, as
they stay the same for the lifetime of the program, but not
for anything else.

Even for string literals, it's tricky, since two equivalent
string literals might not have the same address; e.g.:

std::map<char const*, Something> myMap;
myMap.insert(std::make_pair("key", Something()));
assert(myMap.find("key") != myMap.end());

could fail.