0

The function I am working on is to read in a first name, last name, and four float test scores from a file. I have used the cin and cout operators to check that everything is reading in properly for the first iteration through the while loop. I am checking to make sure that the first name is not the same as "No", which is the sentinel in this case. I have attached my read function, but can easily attach other information that may be necessary.

void ReadInput(ifstream &infile,  char *First[], char *Last[], float scores[50][5], int &REU, int &CEU)
    //Recieves - the input file, array of first names, last names, array of test scores, and rows and columns used
    //Task - reads in the names and test scores from a file
    //Returns - filled arrays of first and last names and test scores

    //cout << "now in ReadInput " << endl;
    char *newPtr;
    char first[15], last[15];
    int i;
    float score;
    REU = 0;
    infile >> ws;
    infile.getline(first,15); //read first name from file
    while(strcmp(first,"No            ") != 0) // read in names and test scores until "No More"
    {
        newPtr = new char[15];
        strcpy(newPtr, first);
        First[REU] = newPtr;
        infile >> ws;
        infile.getline(last,15);
        newPtr = new char[15];
        strcpy(newPtr, last);
        Last[REU] = newPtr;
        CEU = 0; //Initialize the columns used for use in the next row
        for(i=0; i<4; i++) //loop through each column in file
            {
            infile >> scores[REU][i]; //read in the test score
            }
        REU++;
        infile >> ws;
        infile.getline(first,15);
        CEU=i;
    }
    cout << "Now Leaving Read" << endl;
    cin.ignore();
    return;
}
  • 5
    Why are you you dynamically allocating fixed size strings? Why aren't you using `std::string`? – Neil Kirk Oct 30 '15 at 20:09
  • 5
    Spaces are not wildcards in C++. Unless `first` ever becomes `"No "` your loop will never end – NathanOliver Oct 30 '15 at 20:10
  • @NathanOliver In the file, "No" is the marker that tells me to stop reading in information. Isn't the strcmp comparing the length of first to "No"? Since first is technically 15 characters, No had to be followed by empty characters since the comparison is on the length. – Carly Fristoe Oct 30 '15 at 20:16
  • @NeilKirk my professor explicity told us that each name should be initialized to 15 characters. – Carly Fristoe Oct 30 '15 at 20:19
  • 15 including or not including the terminating null? – user4581301 Oct 30 '15 at 20:20
  • @user4581301 sorry for not clarifying, 15 including the null character. – Carly Fristoe Oct 30 '15 at 20:21
  • isn't it "No More" instead of `"No "` ? The comment at least tells that. – decltype_auto Oct 30 '15 at 20:21
  • 1
    @decltype_auto It is "No More", however "No" is technically a first name, and "More" is a last name, so I figured I could end the reading as soon as I hit the first name. – Carly Fristoe Oct 30 '15 at 20:22
  • @decltype_auto the file is spaced out as so: First Name (new line) Last Name (new line) four test scores so the line that says No (new line) More (new line) is the sentinel that tells me to stop reading in information. – Carly Fristoe Oct 30 '15 at 20:24
  • 1
    Even if it is exactly 15 characters, you should still use `std::string` because it is easier, safer and doesn't leak memory. This is what businesses are most interested in when you code! – Neil Kirk Oct 30 '15 at 20:24
  • Using a dummy value to end your input is a very bad idea. All it takes is 1 person in the nearly 7 billion in on this planet to be called "No More" (or claim they are), and your program is invalid! – Neil Kirk Oct 30 '15 at 20:26
  • @NeilKirk I understand that using that type of value is not a good idea, but my professor created the file, so I don't have much control over that for this project. – Carly Fristoe Oct 30 '15 at 20:28
  • End of file sounds like a good file terminator to me. Read until you can't read anymore. – user4581301 Oct 30 '15 at 20:28
  • Either store the number of entries at the beginning of the file in an agreed way, or simply continue until you reach the end of file (if you will always load the whole file anyway) or begin each entry with an agreed value that cannot be confused with a name or data value and stop when you don't find the begin value when you expect it. – Neil Kirk Oct 30 '15 at 20:29
  • @user4581301 I can't do that, because of the specification my professor gave us to end at "No More" – Carly Fristoe Oct 30 '15 at 20:29
  • That is fine.. but I wrote it before your edit.. – Neil Kirk Oct 30 '15 at 20:30
  • Does anyone see issues that would be causing an infinite loop in my code? I realize there are other issues that I need to resolve, but I am really working on getting it to compile right now. – Carly Fristoe Oct 30 '15 at 20:31
  • OK. Sounds to me like the instructor wants you to read "no more" on a single line. – user4581301 Oct 30 '15 at 20:31
  • 1
    Right. Sorry. Got distracted. `infile.getline(first,15)` reads up to 14 characters, then terminates. If the line has only "No" on it, you get a 15 character array containing N, o, null, and God only knows what in the other 12 characters. You will almost certainly not get N, o, and 12 spaces which is that you are looking for in `strcmp(first,"No ")` – user4581301 Oct 30 '15 at 20:34
  • "No" and "More" are on two separate lines, so I am terminating the read function as soon as it reads "No". – Carly Fristoe Oct 30 '15 at 20:34
  • If you read the line `"No"`, it will not contain `"No "` (<- many spaces that don't show up here) in the string, so your comparison is invalid. – Neil Kirk Oct 30 '15 at 20:36
  • First, you have to make sure that theres is not a tab instead of spaces in your code. Then, keep in mind that files in text mode doe not guarantee that trailing witespaces at end of file are not truncated (see [here](http://stackoverflow.com/a/20864389/3723423)) – Christophe Oct 30 '15 at 20:36
  • 1
    Being a jerk, If I were the instructor I'd have at least one person named No in the file to see who terminated early and failed the assignment. Best to always look for both No and more – user4581301 Oct 30 '15 at 20:37
  • Thank you everyone for your help. I changed the comparison, which made it compile. – Carly Fristoe Oct 30 '15 at 20:38

1 Answers1

0

OP is under some rigid bounds from the looks of things and is not allowed to use std::string or similar other fun tools that make this task trivial. Pity.

OK The problem: Infinite loop.

Why?

Two reasons:

  1. All of the file reads go unchecked, so when the file hits the end, or a read of a float that fails to convert, the file stream enters and stays in the error state forever and can never read the exit condition of "No\nmore".

  2. Misinterpreting how getline works and comparing too long a string against garbage.

    infile.getline(first,15);

Reads to the ned of the line and stores up to 14 characters before tacking on a null. If the line ends early, getline places the null. So the target line of No will place No\0 into first

while(strcmp(first,"No            ") != 0) 

will compare No\0 against No \0. These do not match.

As this is an assignment, I will only briefly cover the solution

while(infile.getline(first,15) && 
      infile.getline(last,15) &&
      read in the rest of the inputs )
{
    if (strcmp(first, "No") == 0 &&
        strcmp(last, "more") == 0)
    {
        break;
    }
    all inputs are good, so process everything
}
user4581301
  • 33,082
  • 7
  • 33
  • 54