Fstream not reading file properly

Question

Im trying to write a program that will read a file and set a name and the 5 votes the person casts. The problem is that the fstream is not reading the file correctly. However if I open another object in the main the method they both print perfectly.

I've tryed looking at similar issues but none seem to fix it.

#include <iostream>
#include <fstream>
using namespace std;

class pledge {
        public:
                string name;
                string *vote;
                void setName(string n) {name = n;}
                void setVote(string *v) {vote = v;}
                void printName() {cout << name << endl;}
                void printVote();
                void print();
};

void pledge::printVote() {
        cout << vote[0] << endl;
        cout << endl;
}
void pledge::print() {
        cout << name << ": ";
        for (int i = 0; i < 5; i++) {
                cout << vote[i] << ", ";
        }
        cout << endl;
}

pledge* createPledges(char[], int);

int main(int agrc, char *argv[]) {
        int n;
        n = stoi(argv[2]);
        pledge *p = createPledges(argv[1], n);
        cout << p -> vote[0] << endl;
}

pledge* createPledges(char filename[], int n) {
        fstream input;
        input.open(filename);
        pledge* p = new pledge();
        string name, vote[5];
        getline(input, vote[0]);
        p -> setVote(vote);
        return p;
}

Answer 1

One of the biggest reasons that fstream is not working is because the votes array is not implemented correctly. This answer gives a few tips on how to correct that.

I am going to assume that this is a homework assignment, and also that you have no experience with some of the advanced (and not-so-advanced) C++ features that could make the assignment easier.

• No std::vector
• No std::array
• No operator functions like operator<< and operator[]
• No base-member initialization list
• No throw

Okay, so use a built-in array to hold votes. Do not use a pointer.

class Pledge
{
    // This class manages the data for a single pledge (and his 5 votes).

    enum : std::size_t { n_votes = 5u };
    std::string name;
    std::string votes[n_votes];  // use a built-in array to store votes.

public:
    // ...
};

Suppose you want two constructors, one that takes a name only, and another which takes a name, and an array of five votes. The first one should be easy enough, but the second will require copying the array of votes. You have to pass the array (and its size), and then run a loop to do the copying.

Here is an example that is careful to match size of the array that is passed into the constructor against the size of the array stored in a Pledge. If they are not the same, it avoids accessing any out-of-range elements.

public:
    Pledge(
        std::string const& name,
        std::string votes[],
        std::size_t const size)
    {
        this->name = name;

        // We can take up to `n_votes` votes.
        // Anything beyond that, we have to discard.
        std::size_t n_elements_to_copy = n_votes;
        if (size > n_votes)
        {
            std::cout << "ERROR: Expecting " << +n_votes << " votes, "
                "but " << size << " were passed into the constructor "
                "of class Pledge. The extras have been discarded.\n";
        }
        else if (size < n_votes)
        {
            n_elements_to_copy = size;
            std::cout << "ERROR: Expecting " << +n_votes << " votes, "
                "but only " << size << " were passed into the constructor "
                "of class Pledge.\n";
        }
        for (std::size_t i = 0; i < n_elements_to_copy; ++i)
            this->votes[i] = votes[i];
    }

This works, and might be worth keeping, but I have another idea. What if we just hard-code the five votes into the constructor? It's almost like cheating, but the code is stupid-simple.

    Pledge(
        std::string const& name,
        std::string const& vote0,
        std::string const& vote1,
        std::string const& vote2,
        std::string const& vote3,
        std::string const& vote4)
    {
        this->name = name;
        votes[0] = vote0;
        votes[1] = vote1;
        votes[2] = vote2;
        votes[3] = vote3;
        votes[4] = vote4;
    }

Next, we should take a look at the set and get routines used for votes. They should accept a subscript as an argument, and check to make sure it is not out-of-range.

    void set_vote(std::size_t const i, std::string const& v)
    {
        if (i < n_votes)
            votes[i] = v;
        else
            std::cout << "ERROR - subscript out of range";
    }

    std::string get_vote(std::size_t const i) const
    {
        if (i < n_votes)
            return votes[i];
        std::string msg{ "ERROR - subscript out of range" };
        std::cout << msg << '\n';
        return msg;
    }

You pretty much have the print routines covered. Just two tips:

1. You can avoid the extra comma by printing the first vote outside the loop (at the top), and then start the loop with 1, instead of with 0.
2. The overall print routine should call print_name, followed by print_votes, and let them do the work.

I think you should do a complete rewrite of function createPledges (or even delete it). I would start by coding a function that reads exactly one Pledge and his five votes, and then returns it directly. Do not use any pointers or operator new.

Pledge readOnePledge(std::istream& ist)
{
    // `ist` is the `std::ifstream` object you create (and open) elsewhere.
    // When you open a file, you have to use `ifstream` (or `fstream`).
    // When you read from that file, it is okay to use either `istream` 
    // or `ifstream`. They both work.

    std::string name, vote0, vote1, vote2, vote3, vote4;

    // Read name and votes...

    // And then use the constructor to create (and return) a `Pledge`
    // object.
    return Pledge(name, vote0, vote1, vote2, vote3, vote4);
}

I have left many details incomplete. That is intentional. This should give you a good start, but I want to let you have some of the fun, too!