C++ Runtime Issue?

[wildgoosespeeder]

#include <iostream>
#include <fstream>
using namespace std;

//prototypes
int readRoster(char[][20], int[], int[]);
void printRoster(char[][20], int[], int[], int);
void sortNumber(char[][20], int[], int[], int);
void sortHR(char[][20], int[], int[], int);

//global variables
const int SIZE = 20;

void main()
{
char name[SIZE][SIZE];
int number[SIZE], hr[SIZE];
int numPlayers = readRoster(name, number, hr);
printRoster(name, number, hr, numPlayers);
sortNumber(name, number, hr, numPlayers);
printRoster(name, number, hr, numPlayers);
sortHR(name, number, hr, numPlayers);
printRoster(name, number, hr, numPlayers);
}
int readRoster(char name[][20], int number[], int hr[])
{
ifstream file;
char input[10];
cout << "Enter file name: ";
cin >> input;
file.open(input);
if(file.fail())
{
cout << "Error. File not found." << endl;
return 0;
}
int count = 0, mode = 0;
char temp[SIZE];
while(!file.eof())
{
file >> temp;
if(temp[0] != '\0')
{
char firstName[SIZE];
char lastName[SIZE];
//I Googled how to parse strings to ints using the atoi function.
if(mode == 0)
{
number[count] = atoi(temp);
}
if(mode == 1)
{
strcpy(firstName, temp);
}
if(mode == 2)
{
strcpy(lastName, temp);
strcat(name[count], "\t");
strcat(name[count], firstName);
strcat(name[count], "\t");
strcat(name[count], lastName);
strcat(name[count], "\t");
}
if(mode == 3)
{
hr[count] = atoi(temp);
}
mode++;
if(mode == 4)
{
count++;
mode = 0;
}
}
}
file.close();
cout << "\nRoster:\n";
return count;
}
void printRoster(char name[][20], int number[], int hr[], int numPlayers)
{
cout << "numPlayers = " << numPlayers << endl;
for(int i = 0; i < numPlayers; i++)
{
cout << number[i] << name[i] << hr[i] << endl;
}
cout << endl;
}
void sortNumber(char name[][20], int number[], int hr[], int numPlayers)
{
cout << "Sorted by number:\n";
bool swapped = true;
int tmp;
char temp[SIZE];
while(swapped)
{
swapped = false;
for(int i = 0; i < numPlayers - 1; i++)
{
if(number[i] > number[i+1])
{
tmp = number[i];
number[i] = number[i+1];
number[i+1] = tmp;
tmp = hr[i];
hr[i] = hr[i+1];
hr[i+1] = tmp;
strcpy(temp, name[i]);
strcpy(name[i], name[i+1]);
strcpy(name[i+1], temp);
swapped = true;
}
}
}
}
void sortHR(char name[][20], int number[], int hr[], int numPlayers)
{
cout << "Sorted by homeruns:\n";
bool swapped = true;
int tmp;
char temp[SIZE];
while(swapped)
{
swapped = false;
for(int i = 0; i < numPlayers - 1; i++)
{
if(hr[i] > hr[i+1])
{
tmp = hr[i];
hr[i] = hr[i+1];
hr[i+1] = tmp;
tmp = number[i];
number[i] = number[i+1];
number[i+1] = tmp;
strcpy(temp, name[i]);
strcpy(name[i], name[i+1]);
strcpy(name[i+1], temp);
swapped = true;
}
}
}
}

Somehow this code displays nothing after the file input but should really be displaying:

rctaddict.adryheat.net/Chap12.PNG

Use the DAT files included in the ZIP for input:
rctaddict.adryheat.net/input.zip

What am I doing wrong? My C++ teacher couldn't figure it out!

[Jordan]

Add the red. You're experiencing a buffer overflow. 1024 may be an over kill, but just be sure to use a power of 2. Also, standard C++ requires the main function to return an integer. I recommend turning on the -Wall and -pedantic flags so you get warnings for those types of things (it wouldn't even let me compile).

By the way, how long did your teacher look at your code? It wasn't hard to narrow it down with the debugger. You just have to figure out which function is causing the error, then go into the function and figure out which line it is. In the case where you have a lot of looping such as in your code, you'll want to add break points with conditions. When I was debugging your code, I set a break point to "break" when i == 12. Tricks like this will help you quickly find the error (and you'll have a lot of errors in C++ since you have to handle a lot of the memory yourself).

void sortNumber(char name[][20], int number[], int hr[], int numPlayers)
{
      cout << "Sorted by number:\n";
      bool swapped = true;
      int tmp;
      //char temp;
      char temp[1024];
      while(swapped)
      {
            swapped = false;
            for(int i = 0; i < numPlayers - 1; i++)
            {
                  if(number > number[i+1])
                  {
                        tmp = number;
                        number = number[i+1];
                        number[i+1] = tmp;
                        tmp = hr;
                        hr = hr[i+1];
                        hr[i+1] = tmp;
                        strcpy(temp, name);
                        strcpy(name, name[i+1]);
                        strcpy(name[i+1], temp);
                        swapped = true;
                  }
            }
      }
}
void sortHR(char name[][20], int number[], int hr[], int numPlayers)
{
      cout << "Sorted by homeruns:\n";
      bool swapped = true;
      int tmp;
      //char temp;
      char temp[1024];
      while(swapped)
      {
            swapped = false;
            for(int i = 0; i < numPlayers - 1; i++)
            {
                  if(hr > hr[i+1])
                  {
                        tmp = hr;
                        hr = hr[i+1];
                        hr[i+1] = tmp;
                        tmp = number;
                        number = number[i+1];
                        number[i+1] = tmp;
                        strcpy(temp, name);
                        strcpy(name, name[i+1]);
                        strcpy(name[i+1], temp);
                        swapped = true;
                  }
            }
      }
}

[wildgoosespeeder]

Nope, that didn't do it. I doubted it would anyway. Before the code I posted, the readRoster, sortNumber, and sortHR functions didn't have me passing in arrays as parameters but rather have the arrays in the main function be global and the code worked fine. Problem is I can't have that.

My teacher used some debug tools and the code is reading in from the file correctly.

[Jordan]

The program has no run time errors on my computer when I modify your code. Is it crashing or are you just getting the incorrect output?

I'm compiling with the GNU GCC compiler. Try completely rebuilding your project if you haven't already since sometimes the executable or dll won't get correctly overwritten. Here's my output which appears to be correct except for some extra ASCII characters which shouldn't be a difficult problem to solve.

Roster:
numPlayers = 14
12@öG Alfonso Soriano Derek Lee 33
25 Derek Lee 22
2↑₧n Ryan Theriot 3
16ê²( Aramis Ramirez 26
7h↨G Mark DeRosa 10
11ñDG Jaque Jones 5
15h↨G Cliff Floyd 9
19♠ Matt Murton 8
17ö¥G Mike Fontenot 3
20 Felix Pie 2
18 Jason Kendall 1
29ö¥G Angel Pagan 4
32ö¥G Daryl Ward 3
24☺ Henry Blanco 0

Sorted by number:
numPlayers = 14
2↑₧n Ryan Theriot 3
7h↨G Mark DeRosa 10
11h↨G Mark DeRosa 5
12@öG Alfonso Soriano h↨G Mark DeRosa 33
15h↨G Mark DeRosa 9
16↑₧n Ryan Theriot 26
17ö¥G Mike Fontenot 3
18 Jason Kendall 1
19♠ Matt Murton 8
20 Felix Pie 2
24☺ Henry Blanco 0
25 Derek Lee 22
29ö¥G Angel Pagan 4
32ö¥G Daryl Ward 3

Sorted by homeruns:
numPlayers = 14
24h↨G Mark DeRosa 0
18h↨G Mark DeRosa 1
20h↨G Mark DeRosa 2
2↑₧n Ryan Theriot 3
17h↨G Mark DeRosa 3
32h↨G Mark DeRosa 3
29h↨G Mark DeRosa 4
11h↨G Mark DeRosa 5
19h↨G Mark DeRosa 8
15h↨G Mark DeRosa 9
7h↨G Mark DeRosa 10
25h↨G Mark DeRosa 22
16h↨G Mark DeRosa 26
12@öG Alfonso Soriano h↨G Mark DeRosa 33

[wildgoosespeeder]

Just incorrect output. I am using Microsoft Visual C++ 2008 to compile this. The extra ASCII characters could be because I'm using "\t" to insert tabs and maybe GNU GCC doesn't like that. I delete every file except for the vcproj, input, and cpp files and that still doesn't do it.

[Jordan]

I think we may have been experiencing different errors. When I ran your code after directly copying and pasting it, it would crash during runtime because of a buffer overflow which is what I thought you meant by "runtime issue". Did you even have that error? Or are you simply experience a logical error where it's giving the wrong output or no output at all?

I compiled your code with Visual Studio and I now see what you mean. I'll take another look at it tomorrow, but it looks like you're just screwing up somewhere with the string functions and writing into memory outside of the bounds of the array (which C++ will let you do).

[wildgoosespeeder]

Apr 5, 2011 20:48:13 GMT -8 Jordan said:

I think we may have been experiencing different errors. When I ran your code after directly copying and pasting it, it would crash during runtime because of a buffer overflow which is what I thought you meant by "runtime issue". Did you even have that error? Or are you simply experience a logical error where it's giving the wrong output or no output at all?

I compiled your code with Visual Studio and I now see what you mean. I'll take another look at it tomorrow, but it looks like you're just screwing up somewhere with the string functions and writing into memory outside of the bounds of the array (which C++ will let you do).

I have no idea really why such a freak thing is occurring. I get no crash just a "Press any key to continue..." after I enter in the input file.

That's what I hate about the char data type. The string class is so much easier and not so picky but the class hasn't gotten there yet.

[Jordan]

Apr 6, 2011 0:18:39 GMT -8 wildgoosespeeder said:

I have no idea really why such a freak thing is occurring. I get no crash just a "Press any key to continue..." after I enter in the input file.

Since I'm getting different errors than you, my recommendation is to set a break point in your main function, and then step through each function. Figure out where it's screwing up. Be sure to have the window up that shows the variable's values.

Apr 6, 2011 0:18:39 GMT -8 wildgoosespeeder said:

That's what I hate about the char data type. The string class is so much easier and not so picky but the class hasn't gotten there yet.

Well char arrays are simply contiguous memory terminated with the null terminator. It's not an object so it doesn't have any features at all. It doesn't even have to be interpreted as a char array since it's just raw memory. The string class is actually used a lot, but since it's not a native data type like in Java, it's common for people to just write their own since all it is internally is a char array that grows when it needs to. It's basically an untemplated vector with string-specific functions.

[wildgoosespeeder]

OK, what I was able to find is it's the readRoster function is causing me grief somehow because entering a file name that doesn't exist executes the code correctly.

[wildgoosespeeder]

Forget it! I figured it out! I feel so stupid! One line fixed made it work finally! I tried it out of the blue!

if(mode == 2)
{
strcpy(lastName, temp);
strcat(name[count], "\t");
strcat(name[count], firstName);
strcat(name[count], "\t");
strcat(name[count], lastName);
strcat(name[count], "\t");
}

To:

if(mode == 2)
{
strcpy(lastName, temp);
strcpy(name[count], "\t");
strcat(name[count], firstName);
strcat(name[count], "\t");
strcat(name[count], lastName);
strcat(name[count], "\t");
}

Lock it please!

[Jordan]

Hah, that's definitely a problem. ;P And that's why working with char arrays sucks.