C++ help?

[James Muehlner]

Here's my review of your code. Keep in mind that I haven't taught a programming class in about 10 years so don't take it personally if I'm direct in my points.

From a high level perspective, is there a reason you're not using the C++ String class and the stl containers vector or map? Using them could make your code much easier to write. If your class doesn't prevent you from doing so, I would strongly urge you to look into using the stl container classes and String to do the operations you are trying to do.

Mid-level problems I see:

• First 'passengers[j] = "";' will not work because you can't copy c strings that way. You should probably be assigning NULL instead of ""; "" is the memory location of the char* "\0" at that point in the code.[*]Second, you need to allocate strlen + 1 to get the string in properly, OR strip the newline from the readLine variable before strcpy'ing to name. [*]Third, you're not doing any bounds checking on seatNum and seatLetter or seatLetterNum to make sure that they are within the array bounds.

Low-level nits:

• name is not a very good variable name. Probably seatName would be better.
• readLine is not a very good variable name either (at least for me, because it collidews with the readline library function readline). readBuffer or something similar would be better.
• I don't see readLine decleared/allocated anywhere. Probably easiest would be to allocate it on the state with something like char readLine[1000];
• Instead of substracting 65, it would be clearer if your subtract 'A' since that makes it clear that A->0, B->1 etc.
• If your name variable happens to be as long as 999 characters or longer, the last character will not be a newline, but you'll overwrite it anyway.
• You aren't checking the return values of fgets or fscanf or malloc.
• Your indentation is wrong for the first three lines .

that aside, I don't see anything where it would cause all of the elements in passengers to point to the same memory location (since name gets a fresh malloc everytime through the loop) except for the "" being assigned to passengers[j] during initialization. Perhaps your classes are not being created properly? Do you have any class static member variables?

I'm not using the String class because we have to use char*** for my class.
As far as I can tell,

passengers[i][j] = "";

works. This is convient because later I can compare it using

if(passengers[i][j] == "")

, but I will try using NULL and see if that makes any difference.

I tried using your way, and it didn't seem to help or hurt.

I have changed it to remove the newline from readLine, instead of name, again no noticable difference.

I know I'm not doing any bounds checking in this function, but I can, for class, assume that the input file is valid. I have to keep down the size of my functions to <= 30 lines anyway.

BTW, readLine is allocated in

readLine = (char *) malloc(1000);

How do I check the return value of malloc()? If that was messed up, and it kept serving the same memory over and over, that might do it, no?

I believe that my classes are alright. I'm not doing anything fancy here, and all the other parameters seem to be ok, just not this array.

If you mean using the keyword "static", then no, I don't.

Thanks for the help. If you think of anything else, I'd be very appreciative.

EDIT: Come to think of it, I didn't post the whole function, only excerpts. That would explain that stuff you didn't see. Believe me, it's there.

 

[James Muehlner]

for some reason, repeating the line

passengers = (char ***) (malloc(numRows * sizeof(char **)));

As in

passengers = (char ***) (malloc(numRows * sizeof(char **)));
passengers = (char ***) (malloc(numRows * sizeof(char **)));

works.

It looks like the first one really was serving up the same memory, and the second one reset it to REAL new memory or something. I don't get it.

What is going on here?