reading file into vectors, what's wrong with my input function?

Question

I have an assignment to create a record management system for a class project. When adding records I would like to first read into a vector the contents of my record file currently then perform additions to the file finally outputting back to the record file. However, I'm having a hard time wrapping my mind around how to structure this. I am currently using a dynamic array to store the data but when I try to put it into the vector I it won't let me because it's a pointer. I feel like I'm approaching this entirely wrong and could use some assistance. Here is my input function:

 void student::input(istream& inF, student* stud, vector<student>& vect, int size)
{
    //local variables
    string first, middle, last, addressNum, addressStreet, 
        phone, gender, email, emContactFirst, emContactLast;
    int ID, age;
    string ph, emPhone;
    while (inF)
    {
        for (int index = 0; index < size; index++){
            inF >> first >> last >> middle;
            stud->setName(first, last, middle);
            inF >> ID;
            stud->setId(ID);
            inF >> age;
            stud->setAge(age);
            inF >> phone;
            stud->setPhone(phone);
            inF >> addressNum >> addressStreet;
            stud->setAddress(addressNum, addressStreet);
            inF >> gender;
            stud->setGender(gender);
            inF >> email;
            stud->setEmail(email);
            inF >> emPhone;
            stud->setEmPhone(emPhone);
            inF >> emContactFirst >> emContactLast;
            stud->setEmContact(emContactFirst, emContactLast);
            inF >> stud->gpa >> stud->hobbies >> stud->major
                >> stud->probation;
            if (inF.eof())
                break;
            else
            stud++;
            vect.push_back(stud);
        }
    }
}

`if (inF.eof()` is wrong (you skip the last student) and `student* stud` is code smell — , Nov 04 '15 at 18:33
Don't pass in the object you're using as the target for reading in as a pointer--since you're writing over it multiple times anyway I suspect you don't care about the contents of the base variable when the function is finished. In that case, just make the temporary in the function itself and save yourself some trouble. Oooor you could do `vect.push_back(*stud)` — jaggedSpire, Nov 04 '15 at 18:33
@jaggedSpire So you mean create a temporary pointer inside the function rather than pass a pointer in? — kensai01, Nov 04 '15 at 18:38
@DieterLücking: Skip the last student? Don't you mean, obtain an extra student? — Lightness Races in Orbit, Nov 04 '15 at 18:42
@LightnessRacesinOrbit No that eof-check may prevent, that a well defined student becomes a part of the resulting students. — , Nov 04 '15 at 18:47
@DieterLücking: Oh, right. What's that doing before the push back. Weird. — Lightness Races in Orbit, Nov 04 '15 at 18:56
@kensai01 no, make a local object, and then use `stud.` instead of `stud->` — jaggedSpire, Nov 04 '15 at 18:56
I highly recommend overloading `operator>>` in your `class` or `struct`. This would allow a syntax like: `inF >> *stud;`. — Thomas Matthews, Nov 04 '15 at 19:31

score 2 · Accepted Answer · edited May 23 '17 at 12:23

2

Problems I see:

You are using while (inF) to break the loop. See Why is iostream::eof inside a loop condition considered wrong?.
You are using one pointer, stud to read all the values and storing the same pointer multiple times in vect. First of all, the compiler should produce an error. You cannot add a pointer to a vector of objects.

It's not clear why the function needs stud as an argument. You can just as easily use a local object in the function. Like this:
```
for (int index = 0; index < size; index++){
   student stud;
   if ( !(inF >> first >> last >> middle) )
   {
      // Deal with error.
   }
   stud.setName(first, last, middle);

   ...
}
```
It's better to check whether the calls to inF >> ... assigned anything successfully and not assume that it succeeded. Instead of:
```
inF >> first >> last >> middle;
```
use
```
if ( !(inF >> first >> last >> middle) )
{
    // Deal with error.
}
```
I suggest changing all such calls.

edited May 23 '17 at 12:23

Community

1
1

answered Nov 04 '15 at 18:41

R Sahu

204,454
14
159
270

Actually, (1) is OK. Also trying to read multiple fields (unchecked) is OK, when a final test `if(stream) ` follows. – Nov 04 '15 at 18:53
@R Sahu How do I go about changing #2, that was the crux of my entire question. I'm trying to load from a simple text file that has rows of text input pertaining to attributes of the student. I create an array of that many(rows in file) objects of class student. I then fill those objects with the information. Now how do I go to vectors from there or should I even bother using vectors to store such data? Problem is, other requirements of the project are to be able to sort, add and delete and modify individual entries. I'm having a hard time visualizing how to go about implementing this. – kensai01 Nov 04 '15 at 18:55
@DieterLücking, in this case, it is probably a logical error to rely on `while(inF)` to break the loop. – R Sahu Nov 04 '15 at 18:56

reading file into vectors, what's wrong with my input function?

1 Answers1