Exception thrown at 0x5914F3BE (ucrtbased.dll)

Question

I have some code that takes a list of names + double values from a .txt file and displays these in the command prompt. For this an array of structs is dynamically allocated. The code should know the size of the array based on the first value in the .txt file, which is then followed by the names and associated values. It should then display the list in two parts with names that have an associated double value higher than or equal to 10.000 listed first. If none of the values qualifies, it displays 'None' in the first half.

The program executes, but the debugger gives an exception and the output is not as expected.

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

struct donor
{
    string name;
    double contribution = 0;
};

int main()
{
    string filename;
    ifstream inFile;

    cout << "Enter name of data file: ";
    cin >> filename;

    inFile.open(filename);
    cin.clear();

    if(!inFile.is_open())
    {
        cout << "Could not open the file " << filename << endl;
        cout << "Program terminating.\n";

        exit(EXIT_FAILURE);
    }

    int amount;
    inFile >> amount;

    cin.clear();

    donor* dlist = new donor[amount];
    int i;

    while(inFile.good())
    {
        for(i = 0; i < amount; i++)
        {
            getline(inFile, dlist[i].name);
            cin.clear();

            inFile >> dlist[i].contribution;
            cin.clear();
        }
    }

    cout << "Here's the list of Grand Patrons:\n";
    bool grandpatrons = false;
    for(i = 0; i < amount; i++)
    {
        if(dlist[i].contribution >= 10000)
        {
            grandpatrons = true;

            cout << dlist[i].name << endl;
            cout << dlist[i].contribution << endl;
        }
    }

    if(grandpatrons == false)
    {
        cout << "None" << endl;
    }

    cout << "Here's the list of Patrons:\n";
    for (i = 0; 1 < amount; i++)
    {
        if (dlist[i].contribution < 10000)
        {
            cout << dlist[i].name << endl;
            cout << dlist[i].contribution << endl;
        }
    }

    delete[] dlist;
    return 0;
}

The donorlist.txt file looks like this:

4

Bob

400

Alice

11000

But the output looks like this:

Enter name of data file: donorlist.txt

Here's the list of Grand Patrons:

None

Here's the list of Patrons:

0

0

0

0

The exception that the debugger gives me is:

Exception thrown at 0x5914F3BE (ucrtbased.dll) in 6_9.exe: 0xC0000005: Access violation reading location 0xA519E363.

Now I assume something is going wrong with reading from the dynamically allocated memory. Maybe something is causing me to read from memory beyond the allocated array? I'm having trouble finding exactly where the mistake is being made.

Answer 1

Your problems begin with the wrong amount written in your data file. Fix it with:

2
Bob
400
Alice
11000

They then continue with the fact that you inccorectly read the file.
Remember: Mixing operator>> and getline() is not as simple as it seems.
You see, operator>> IGNORES newline and space characters until it finds any other character.
It then reads the upcoming characters until it encounters the next newline or space character, BUT DOES NOT DISCARD IT.

Here is where the problem with getline comes in. getline reads EVERYTHING until it encounters newline or a specified delim character. Meaning, that if your operator>> stops after encountering newline, getline will read NOTHING since it immediately encounters newline.

To fix this, you need to dispose of the newline character. You can do this by first checking if the next character in the stream is indeed newline and then using istream::ignore() on it;

int next_char = stream.peek();
if(next_char == '\n'){
    stream.ignore();
}

A working example of your code would be:

#include <iostream>
#include <fstream>
#include <string>

using namespace std;

//Suggestion: class/struct names should start with a capital letter.
struct Donor{
    //Suggestion: Use member initializer lists to specify default values.
    Donor() : name(), contribution(0){}

    string name;
    double contribution;
};

int main(){
    cout << "Enter the filename: ";

    string filename;
    cin >> filename;

    //Suggestion: Open the file immediately with the filename and use `operator bool` to check if it opened.
    ifstream inFile(filename);
    if(!inFile){
        cout << "Could not open the file " << filename << '\n';
        cout << "Program terminating.\n";

        exit(EXIT_FAILURE);
    }

    int amount;
    inFile >> amount; //! Leaves '\n'

    Donor* donors = new Donor[amount];

    for(int i = 0; i < amount; ++i){
        switch(inFile.peek()){
            case '\n': inFile.ignore();
                       break;

            case  EOF: cout << "Donor amount too big!\n";
                       exit(EXIT_FAILURE); 
        }

        getline(inFile, donors[i].name);
        inFile >> donors[i].contribution;
    }

    cout << "Here's the list of Grand Patrons:\n";
    bool grandpatrons_exist = false;
    for(int i = 0; i < amount; ++i){
        if(donors[i].contribution >= 10000){
            grandpatrons_exist = true;

            cout << donors[i].name << '\n';
            cout << donors[i].contribution << '\n';
        }
    }

    if(!grandpatrons_exist){
        cout << "None\n";
    }

    cout << "Here's the list of Patrons:\n";
    for(int i = 0; 1 < amount; ++i){
        if(donors[i].contribution < 10000){
            cout << donors[i].name << '\n';
            cout << donors[i].contribution << '\n';
        }
    }

    delete[] donors;
    return 0;
}

Now, an even better solution would be to use vectors instead of raw pointers and implement operator>> and operator<< which would greatly simplify the reading and printing of the objects.

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <algorithm>

using namespace std;

class Donor{
    public:
        Donor() noexcept: name(), contribution(0){}

        friend istream& operator>>(istream& stream, Donor& donor){
            switch(stream.peek()){
                case  EOF: return stream;
                case '\n': stream.ignore();
            }

            getline(stream, donor.name);
            stream >> donor.contribution;

            return stream;
        }
        friend ostream& operator<<(ostream& stream, const Donor& donor){
            stream << donor.name << ' ' << donor.contribution;

            return stream;
        }

        const string& get_name() const noexcept{
            return name;
        }
        const double& get_contribution() const noexcept{
            return contribution;
        }

    private:
        string name;
        double contribution;
};

int main(){
    cout << "Enter the filename: ";

    string filename;
    cin >> filename;

    ifstream inFile(filename);
    if(!inFile){
        cout << "Could not open the file " << filename << '\n';
        cout << "Program terminating.\n";

        exit(EXIT_FAILURE);
    }

    int amount;
    inFile >> amount;

    vector<Donor> donors(amount);
    //Read it as `for donor in donors`
    for(Donor& donor : donors){
        inFile >> donor;
    }

    //An STL function that takes a lambda as the thirs argument. You should read up on them if you haven't.
    //I would prefer using this since it greatly improves readability.
    //This isn't mandatory, your implementation of this part is good enough.
    bool grandpatrons_exist = any_of(begin(donors), end(donors), [](const Donor& donor){ return donor.get_contribution() >= 10000; });

    cout << "Here's the list of Grand Patrons:\n";
    if(grandpatrons_exist){
        for(const Donor& donor : donors){
            if(donor.get_contribution() >= 10000){
                cout << donor << '\n';
            }
        }   
    }
    else{
        cout << "None\n";
    }

    cout << "\nHere's the list of Patrons:\n";
    for(const Donor& donor : donors){
        if(donor.get_contribution() < 10000){
            cout << donor << '\n';
        }
    }

    return 0;
}

Some other great improvements would be:

• Use partition to seperate great patrons from normal ones.
• Use stream iterators to read the objects into the vector.

int main(){
    cout << "Enter the filename: ";

    string filename;
    cin >> filename;

    ifstream inFile(filename);
    if(!inFile){
        cout << "Could not open the file " << filename << '\n';
        cout << "Program terminating.\n";

        exit(EXIT_FAILURE);
    }

    //Ignore the first line completely
    inFile.ignore(numeric_limits<streamsize>::max(), '\n'); 
    //Calls `operator>>` internally
    vector<Donor> donors(istream_iterator<Donor>{inFile}, istream_iterator<Donor>{});

    auto first_grand_patron = partition(begin(donors), end(donors), [](const Donor& donor){ return donor.get_contribution() >= 10000; });

    cout << "Here's the list of Grand Patrons:\n";
    if(first_grand_patron == begin(donors)){
        cout << "None!\n";
    }
    for(auto patron = begin(donors); patron != first_grand_patron; ++patron){
        cout << *patron << '\n';
    }

    cout << "\nHere's the list of Patrons:\n";
    for(auto patron = first_grand_patron; patron != end(donors); ++patron){
        cout << *patron << '\n';
    }

    return 0;
}

Now some general tips:

• Struct/Class names should start with a capital letter.
• Stop Using std::endl.
• No need to cin.clear(). Cin is only used once and never again.
• Use member-initializer lists.
• Optionally use ++i instead of i++ in for loops to get used to the correct way of incrementing a variable unless needed otherwise.
• bool grandpatrons is too much of an abstract name for a flag.
• donors is a subjectively better name than short for donor list.