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:
- You have arrived at the end of the file before the second loop starts.
- 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";
}
}