Using overloaded << operator with dynamic array

Question

My problem as follows:

I want to read values from a file into a dynamic array of specific class called Voter, using an overloaded >> operator. Then, i want to export my result to the screen to check my load with another overloaded << operator.

Using a normal array I get the result im after, but struggling with going dynamic. I have read on the forums that vectors are maybe a better way to go, but I want to understand what I'm doing wrong on this one.

Thanks!

Details:

The file has 3 "values" seperated by a space: id(string) nr(int) (1 or 0)bool

Code extract:

string RollFileName = "VotersRoll.dat";
ifstream inFile_VotersRoll;
inFile_VotersRoll.open(RollFileName);

if(inFile_VotersRoll.fail())
{
    printf("The VotersRoll.dat file failed to open.\n");
    exit(1);

}
//connect to in file



int numberOfEntries = 0;
Voter testVoter;
while(inFile_VotersRoll>>testVoter)
{
    numberOfEntries++;
}
//get number of voter objects from file

typedef Voter* VoterArrayPntr;
VoterArrayPntr rollArray;
rollArray = new Voter[numberOfEntries];
//create dynamic arrray for voter objects


while (inFile_VotersRoll>>*rollArray) {
    cout << *rollArray;

}

Class (extract):

class Voter
{
public:
    friend istream& operator>>(istream &inP, Voter &voterP);
    friend ostream& operator<<(ostream &outP,const Voter &voterP);
private:
    string id;
    int nr_times_voted;
    bool voted;
}

Friend functions:

istream& operator>>(istream &inP,Voter &voterP)
{  
   inP >> voterP.id >> voterP.nr_times_voted >> voterP.voted;
   return inP; 
}
ostream& operator<<(ostream &outP,const Voter &voterP)
{ 
   outP << voterP.id << voterP.nr_times_voted << voterP.voted;   
   return outP;
}

Answer 1

In C++, std::vector implements the concept of a dynamic array. This is not just "maybe" a better way. Don't use new[] and delete[]!

(In fact, delete[] is entirely missing from your example code. If you don't release the memory anywhere and the object is not conceptually meant to exist as long as the program runs, then that's a memory leak.)

As for why the code doesn't work here, let's first have a look at this loop:

int numberOfEntries = 0;
Voter testVoter;
while(inFile_VotersRoll>>testVoter)
{
    numberOfEntries++;
}

This is curious; your first read the whole file just to get the number of elements, and will then read it again to get the actual elements?

This is redundant. You should get everything in one loop.

But there are also two serious bugs here:

1. You have arrived at the end of the file before the second loop starts.
2. You use only the first element of your array.

Let's fix the first problem first. In the next loop,

typedef Voter* VoterArrayPntr;
VoterArrayPntr rollArray;
rollArray = new Voter[numberOfEntries];
//create dynamic arrray for voter objects

while (inFile_VotersRoll>>*rollArray) {
    cout << *rollArray;
}

the loop condition will never be true. Your operator>> is called once. The stream is returned from the operator. And finally, it is used in the loop condition, resulting in false – because you had already been at the end of the file.

A very quick fix would be to open a new stream for the same file to fix this:

typedef Voter* VoterArrayPntr;
VoterArrayPntr rollArray;
rollArray = new Voter[numberOfEntries];
//create dynamic arrray for voter objects

ifstream inFile_VotersRoll2; // ugly workaround
inFile_VotersRoll2.open(RollFileName);
while (inFile_VotersRoll2>>*rollArray) {
    cout << *rollArray;
}

This will read and print the whole file. But still, everything will have been read into and written from only the first element of the array. *rollArray refers to the first element, and rollArray never changes.

This second bug is fixed like this:

for (int index = 0; (index < numberOfEntries) && (inFile_VotersRoll2>>rollArray[index]); ++index) { 
    cout << rollArray[index];
}

But as I said before, your goal should be to read the whole thing at once. This implies increasing the array's size as you go. Fortunately, with std::vector, this is trivial, because std::vector does not only grow automatically, it also does so in a clever way. And the memory is released automatically as well. Here we go, your complete example modified to use std::vector:

#include <string>
#include <iostream>
#include <fstream>
#include <vector>
#include <stddef.h>

using namespace std; // just for quick'n'dirty test code

class Voter
{
public:
    friend istream& operator>>(istream &inP, Voter &voterP);
    friend ostream& operator<<(ostream &outP,const Voter &voterP);
private:
    string id;
    int nr_times_voted;
    bool voted;
};

istream& operator>>(istream &inP,Voter &voterP)
{  
   inP >> voterP.id >> voterP.nr_times_voted >> voterP.voted;
   return inP; 
}
ostream& operator<<(ostream &outP,const Voter &voterP)
{ 
   outP << voterP.id << voterP.nr_times_voted << voterP.voted;   
   return outP;
}

int main() {

    string RollFileName = "VotersRoll.dat";
    ifstream inFile_VotersRoll;
    inFile_VotersRoll.open(RollFileName);

    if(inFile_VotersRoll.fail())
    {
        std::cerr << "The VotersRoll.dat file failed to open.\n";
        return EXIT_FAILURE;
    }

    std::vector<Voter> voters;

    Voter input;
    while (inFile_VotersRoll >> input) {
        voters.push_back(input);
    }

    for (auto const &voter : voters) {
        std::cout << voter << "\n";
    }
}

Answer 2

"I want to understand what I'm doing wrong on this one."

Your loop here

while (inFile_VotersRoll>>*rollArray) {
    cout << *rollArray;
}

will only write to the 1st element in the array. You need to increment the pointer to the next element after reading in:

inFile_VotersRoll.seekp(0); // Rewind to the beginning of inFile_VotersRoll

VoterArrayPntr curItem = rollArray;
while (inFile_VotersRoll>>*curItem) {
    cout << *curItem;
    curItem++; // <<<<<<
}