While loop with variable problem

Okay, so I am making a project where a user inputs a different variable to change, delete, print, or add to an array. When they type a q, the program quits. My problem is that when they type two of the same character (say p then another p after the if executes) the array does not print out the second, third.....billionth time. What is the issue here? (I haven't added any other selections yet because I still need to figure out the algorithms for those.)

Code:

---

    cout << "Please enter a selection. \nTo print the array, type P.\n";
    cout << "To change a test score in the array, type C.\n";
    cout << "To add a student to the array, type A.\n";
    cout << "To delete a student from the array, type D.\n";
    cout << "To quit, type Q. \nPlease enter selection now: ";
    cin >> selection;
 
while (selection != 'q' || selection != 'Q')
{
     
    if (selection == 'p' || selection == 'P')
    {
    cout << setw(8) << "Name" << setw(15) << "ID" << setw(20) << "Score\n\n";

    for (counter = 0; inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)
    {
        cout << setw(8) << studentList[counter].Name <<  "\t";
        cout << setw(8) << studentList[counter].ID << "\t";
        cout << setw(8) << studentList[counter].score << "\t" << endl;
        }
        }

    cout << "Please enter a selection. \nTo print the array, type P.\n";
    cout << "To change a test score in the array, type C.\n";
    cout << "To add a student to the array, type A.\n";
    cout << "To delete a student from the array, type D.\n";
    cout << "To quit, type Q. \nPlease enter selection now: ";
    cin >> selection;

}

---

Code:

---

    for (counter = 0; inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)
    {
        cout << setw(8) << studentList[counter].Name <<  "\t";
        cout << setw(8) << studentList[counter].ID << "\t";
        cout << setw(8) << studentList[counter].score << "\t" << endl;
        }

---

Seems to me that you're reading from the file inFile here. That will probably only work once, and then you'll get to the end of the file or something that's an invalid ID or whatever.

Therefore, the first time you execute this loop, it will read in data from the file, and print it out at the same time. All subsequent executions will result in nothing happening, unless you rewind the file.

I think you probably didn't mean to have the inFile>> code in that loop, since 'p' is supposed to print the data.

Once you have read the file, your second loop will encounter a end-of-file immediately.

There are three (make that at least five) approaches you can take:
1. Load the entire file up-front, before you even ask the user what they want to do.
2. Load data on demand, so whenever you need the data from the file, you check if it's been loaded, and if not, read in the entire file then.
3. Read the file each time it's needed, opening and closing the file before/after the read-loop.
4. use fstream::reset() to reset the end-of-file situation and fstream::seekg() to set the file-pointer (where you are next going to read from in the file) to the start of the file.
5. Use mmap/MapViewOfFile to map the file into memory, and use pointer arithmetics to get the data.

I would probably recommend #1, based on your early-stage of learning. It's pretty simple method, it solves/avoids nearly all the potential problems with any of the other solutions with little effort. The only drawback is if the list of students is REALLY huge, and it takes a long time to load them, and the user didn't need to do anything with the students - but that would be the rare variant, don't you think.

--
Mats

To load the file upfront at the beginning of the program I used the following function with a for loop to initialize the data:

Code:

---

void initialize (ifstream& inFile, studentType list[], int listSize)
{
    int counter;

    for (counter = 0; counter < listSize; counter++)
    {
        inFile >> list[counter].Name >> list[counter].ID >> list[counter].score;
        }
}

---

With this at the top of the file after the struct:

Code:

---

void initialize (ifstream& inFile, studentType list[], int listSize)

---

And this within the if where the selection == 'p' or 'P':

Code:

---

initialize (inFile, studentList, listSize)

---

Is this correct?

Yes, except you probably want to add to the loop-condition that you successfully read a line of data [unless listsize is a known value that indicate exactly how much data is in the file, rather than the MAX size of the list].

And I recommend that you call initialize() _BEFORE_ you ask what the user wants to do.

--
Mats

What loop condition is that?

Well, what you had originally would do.

Code:

---

    for (counter = 0; inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)

---

Code:

---

    for (counter = 0; counter < listSize; counter++)
    {
        inFile >> list[counter].Name >> list[counter].ID >> list[counter].score;
        }

---

The one in red - but like skinning cats, there's many different ways to make read-loops.

My point is that if listSize is, say, 500, and you only have 30 items in the file, then reading "nothing" 469 times when you could have bailed out after the 31st time seems a bit sub-optimal - and whilst optimizing code prematurely is a bad thing, writing pessimal code is also a bad thing.

--
Mats

Yeah, the file size varies because we have to use two separate files that have different numbers of students. I still don't know how to make it stop reading at the end of the file. Sorry but I am JUST starting to learn C++ and am having a lot of difficulty understanding some concepts.

Quote:

---

Originally Posted by Todd88

Yeah, the file size varies because we have to use two separate files that have different numbers of students. I still don't know how to make it stop reading at the end of the file. Sorry but I am JUST starting to learn C++ and am having a lot of difficulty understanding some concepts.

---

dwks gave the answer. Of course, unless you are 100% sure that the content will ALWAYS be smaller than listSize, you should ALSO make sure you don't exceed listSize.

--
Mats

. . . so the condition you probably want is something like this:

Code:

---

    for (counter = 0; counter < listSize && inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)

---

The first condition makes sure you don't overrun your array studentList. After all, the file could have more data than your array can hold, which would be a problem if you didn't have this condition. (matsp explained this above.)

The second condition requires a little bit of explanation. When you read data from an istream, with cin>> or inFile>> or whatever, it evaluates to the file stream itself, or to NULL if an error was encountered. It's a good thing that it evaluates to the stream itself -- this way, you can chain several reads to occur at once, with inFile>>this>>that and so on.

When it evaluates to NULL, you have an error. Perhaps the end of the file; perhaps you tried to read a number and all it found was "abc". Regardless, in this case, you just want to stop reading. This condition evaluates to NULL and breaks the loop when an error is encountered.

The second condition is just like this.

Code:

---

    for (counter = 0; (inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score) != NULL; counter++)

---

Okay so I figure everything initializes properly but I am having trouble printing it all out. I used the following for loop but I'm not sure as to what to put for the conditions.

Code:

---

void printIt (studentType list[])
{
    int counter;
   
    cout << setw(8) << "Name" << setw(15) << "ID" << setw(20) << "Score\n\n";

    for (counter = 0; (don't know what to put here); counter++)
    {
        cout << setw(8) << list[counter].Name <<  "\t";
        cout << setw(8) << list[counter].ID << "\t";
        cout << setw(8) << list[counter].score << "\t" << endl;
        }
}

---

You need to know how many students you have -- perhaps you should pass that around too.

It's a different number of students according to the file that the user wants to use.

Well then, after you use a loop like this to initialize the array,

Code:

---

    for (counter = 0; counter < listSize && inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)

---

take note of the value of counter after the loop has finished executing. studentList[counter-1] will be the last element (assuming some were read at all). If you pass counter to other functions, you can use it for the "(don't know what to put here)". Like so.

Code:

---

for(c = 0; c < counter; c ++)

---

Doesn't matter -- you still need to know what it is -- not as a constant, obviously, but as a variable. You counted when you were loading them in, so you should know, right?

Quote:

---

Originally Posted by dwks

Well then, after you use a loop like this to initialize the array,

Code:

---

    for (counter = 0; counter < listSize && inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)

---

take note of the value of counter after the loop has finished executing. studentList[counter-1] will be the last element (assuming some were read at all). If you pass counter to other functions, you can use it for the "(don't know what to put here)". Like so.

Code:

---

for(c = 0; c < counter; c ++)

---

---

This worked but it prints out student [1], [3], [5], [7]....and so on until the end of the file. Why is this?

Either: You put an extra c++ in the loop, or your read-in function is skipping lines.

I think the initialize function is not initializing correctly because I used cout to print the counter variable and it amounted to 9 when it should be 18.

Quote:

---

Originally Posted by Todd88

I think the initialize function is not initializing correctly because I used cout to print the counter variable and it amounted to 9 when it should be 18.

---

Post your initializing loop...

--
Mats

Code:

---

void initialize (ifstream& inFile, studentType list[], studentType studentList[],
                int& counter, int listSize)
{

    for (counter = 0; inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)
    {
        inFile >> list[counter].Name >> list[counter].ID >> list[counter].score;
        }
        cout << counter;
}

---

Code:

---

void initialize (ifstream& inFile, studentType list[], studentType studentList[],
                int& counter, int listSize)
{

    for (counter = 0; inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score; counter++)
    {
        inFile >> list[counter].Name >> list[counter].ID >> list[counter].score;
        }
        cout << counter;
}

---

What does the red, and the green, code do?

--
Mats

The red code sets the boundary for the for loop to read until it reaches the end of the file.

The green code states what is IN the file.

Quote:

---

Originally Posted by Todd88

The red code sets the boundary for the for loop to read until it reaches the end of the file.

The green code states what is IN the file.

---

That may be what you want it to do. We're asking you to read the code so that you see what it actually does.

Okay so we deleted the red code and just put inFile as the boundary and it worked

Quote:

---

Originally Posted by Todd88

Okay so we deleted the red code and just put inFile as the boundary and it worked

---

Except, I think, you will find that you are now getting 1 more in counter than there are items in your file. Cf. your thread from yesterday, and the solution found there.

--
Mats

For the printIt function I just did this:

Code:

---

void printIt (ifstream& inFile, studentType list[], int& counter)
{
    int c;
   
    cout << setw(8) << "Name" << setw(15) << "ID" << setw(20) << "Score\n\n";

    for (c = 0; c < counter - 1; c++)
    {
        cout << setw(8) << list[c].Name <<  "\t";
        cout << setw(8) << list[c].ID << "\t";
        cout << setw(8) << list[c].score << "\t" << endl;
        }
}

---

Yes, and I suppose instead of being careful when using a knife, you just use band-aid's a lot :-)

The correct solution is to put the infile << ... stuff inside the condition of the loop, and NOT have it in the body of the for-loop, so that immediately when you detect that you can't read any more, you exit the loop.

Alternatively, we could use your original loop construct, and then break inside the loop when you can't read any more:

Code:

---

    for (counter = 0; counter < listSize; counter++)
  {
      if (!inFile >> studentList[counter].Name
                            >> studentList[counter].ID
                            >> studentList[counter].score)
      {
          break;
      }
    }

---

--
Mats

Is it wrong to do it my way? Like will it be a bad way down the road after completing the other functions?

Quote:

---

Originally Posted by Todd88

Is it wrong to do it my way? Like will it be a bad way down the road after completing the other functions?

---

I think it's bad, yes. It's probably not strictly wrong to read one too many and then deduct one - but if you MUST do it that way, then do it inside your read-function, not where you are using the resulting counter variable.

--
Mats

I know you guys probably think I am completely dumb in the field of c++ but I need more help :(.

I now need to ADD to the array a name, id and score. I thought I had a brilliant idea as a way to do this but it isn't working. Here is my code:

Code:

---

void addIt (ifstream& inFile, studentType studentList[], int& counter, string& name,
            int& id, int& score2)
{
   
    cout << "\nPlease enter a name: ";
    cin >> name;
   
    cout << "\nPlease enter an ID: ";
    cin >> id;
   
    cout << "\nPlease enter a score: ";
    cin >> score2;
   
    studentList[counter + 1].Name = name;
    studentList[counter + 1].ID = id;
    studentList[counter + 1].score = score2;

    }

---

Can anyone help?

Quote:

---

Originally Posted by Todd88

Can anyone help?

---

We'd be glad to help, but you have to tell us what the problem is.

I figured it all out by myself. Thanks for looking guys!