Why does this crash my program?

[MisterTams]

I don't know why this crashes my program if I have more than 10 items read into the array of objects from a file? I did this exact code in another program just fine and it would stop reading as soon as the limit of the intialized element number was reached for the array of objects. The only difference this time around with this code is that I am now using a friendship to access the private variables and read data into the private variables DIRECTLY....

Here is the file I am reading from..

Groceries.txt

*has 13 items*

Watermelon# 2.99 2
Pepsi-Cola# 0.97 4
Fried Chicken# 3.99 3
SunChips# 2.59 3
Large Steak# 19.99 3
Canned Peas# 0.98 4
Swai Fish# 8.99 3
Catfish Fillets# 9.99 2
Chicken Nuggets# 2.95 5
Hershey Bar# 0.79 12
Hershey Kisses# 0.39 34
Chicken Fingers# 3.99 2
Fish Sticks# 4.59 3 

Main.cpp

#include "GrItem.h"
// function prototype
void PrintReceipt(GrItem[], int);

int main()
{
	const int SIZE = 10;
	int ElementsRead = 0;
	GrItem Groceries[SIZE];
	ElementsRead = ReadFromFile(Groceries, SIZE);

	if (ElementsRead == 0) {
		cout << "No data was read from the file... Exiting..." << endl;
		exit(-2);
	}
	PrintReceipt(Groceries, ElementsRead);
	return 0;
}

int ReadFromFile(GrItem grocery[], const int size) //granted friendship in GrItem.h
{
	fstream inFile;
	string filename;
	int indexTotal = 0;
	string foodname;
	cout << "User, please enter the name of the textfile to read from." << endl;
	cout << "Enter here: ";
	cin >> filename;

	inFile.open(filename, ios::in);

	if (inFile.fail()) {
		cout << "Error opening file! Exiting Program!" << endl;
		exit(-1);
	}

	while (getline(inFile, grocery[indexTotal].name, '#') && indexTotal < size) //acts as a eof bit flag, will terminate loop if no more to read or size limit is reached
	{
		inFile >> grocery[indexTotal].price;
		inFile >> grocery[indexTotal].quantity;
		inFile.ignore(10000, '\n');
		indexTotal++;
	}
	inFile.close();
	cout << "Successfully read data from: " << filename << endl;
	return indexTotal;
}

void PrintReceipt(GrItem groc[], int size)
{
	double totalcost = 0;
	cout << "\nItem" << setw(25) << " " << "Price" << setw(5) << " " << "Qty" << setw(8) << " " << "Cost" << endl;
	cout << "------------------------------------------------------" << endl;

	for (int index = 0; index < size; index++)
	{
		cout << setw(20) << left << groc[index].getName() << setw(14) << right;
		cout << fixed << setprecision(2);
		cout << groc[index].getPrice() << setw(8);
		cout << groc[index].getQuantity() << setw(12);
		cout << groc[index].getCost() << endl;
		totalcost += groc[index].getCost();
	}
	cout << setw(54) << right << "========" << endl;
	cout << "TOTAL" << setw(49) << totalcost << endl;
}

GrItem.h

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

#ifndef GRITEM_H
#define GRTITEM_H

class GrItem
{
private:
	string name;
	double price;
	int quantity;

public:
	//constructors
	GrItem();
	GrItem(string, double, int);
	GrItem(const GrItem&); //copy constructor

	//mutators
	void setName(string);
	void setPrice(double);
	void setQuantity(int);

	//accessors (inline)
	string getName() { return name; }
	double getPrice() { return price; }
	int getQuantity() { return quantity; }
	
	double getCost(); //getCost(), defined in GrItem.cpp

	friend int ReadFromFile(GrItem [], const int); //friend function to process data into array of GrItem objects

};
#endif 

GrItem.cpp

#include "GrItem.h"

//default constructor
GrItem::GrItem()
{
	name = "No Food";
	quantity = 0;
	price = 0.0;
}
//parameterized constructor
GrItem::GrItem(string Name, double Pri, int Qty)
{
	name = Name;
	price = Pri;
	quantity = Qty;
}
//copy constructor
GrItem::GrItem(const GrItem & obj)
{
	name = obj.name;
	price = obj.price;
	quantity = obj.quantity;
}

void GrItem::setName(string Name)
{
	name = Name;
}

void GrItem::setPrice(double Pri)
{
	price = Pri;
}

void GrItem::setQuantity(int Quan)
{
	quantity = Quan;
}

double GrItem::getCost()
{
	return (price * quantity);
}

In particular I am asking about this function...

int ReadFromFile(GrItem grocery[], const int size) //granted friendship in GrItem.h
{
	fstream inFile;
	string filename;
	int indexTotal = 0;
	string foodname;
	cout << "User, please enter the name of the textfile to read from." << endl;
	cout << "Enter here: ";
	cin >> filename;

	inFile.open(filename, ios::in);

	if (inFile.fail()) {
		cout << "Error opening file! Exiting Program!" << endl;
		exit(-1);
	}

	while (getline(inFile, grocery[indexTotal].name, '#') && indexTotal < size) //acts as a eof bit flag, will terminate loop if no more to read or size limit is reached
	{
		inFile >> grocery[indexTotal].price;
		inFile >> grocery[indexTotal].quantity;
		inFile.ignore(10000, '\n');
		indexTotal++;
	}
	inFile.close();
	cout << "Successfully read data from: " << filename << endl;
	return indexTotal;
}

This right here ^^^^ will work fine if it is 10 items in the .txt file or less.
If more than 10, it crashes. I used similar (almost identical code) in another program and it handled more than what was allowed in the array just fine because it would terminate the loop early if an attempt was made to read more than allowed into the array of objects.

[MisterTams]

If I used this as my function ReadFromFile in Main.cpp it will work fine.
It will only read up to the 10th item in the .txt file.

int ReadFromFile(GrItem grocery[], int size) //granted friendship in GrItem.h
{
	fstream inFile;
	string filename;
	int indexTotal = 0;
	string foodname;
	cout << "User, please enter the name of the textfile to read from." << endl;
	cout << "Enter here: ";
	cin >> filename;

	inFile.open(filename, ios::in);

	if (inFile.fail()) {
		cout << "Error opening file! Exiting Program!" << endl;
		exit(-1);
	}

	while (getline(inFile, grocery[indexTotal].name, '#')) //acts as a eof bit flag, will terminate loop if no more to read 
	{
		inFile >> grocery[indexTotal].price;
		inFile >> grocery[indexTotal].quantity;
		inFile.ignore(10000, '\n');
		indexTotal++;

		if (indexTotal >= 10) //will terminate loop if 10 or more items are read into the array
			break;
	}
	inFile.close();
	cout << "Successfully read data from: " << filename << endl;
	return indexTotal;
}

This works... ^^^

Here below, is the sample output when using the .txt file in the original post with this function. ^^^^^^

/*
User, please enter the name of the textfile to read from.
Enter here: C:\Users\Thomas\Desktop\Groceries.txt
Successfully read data from: C:\Users\Thomas\Desktop\Groceries.txt
Total Elements read from array: 10

Item                         Price     Qty        Cost
------------------------------------------------------
Watermelon                    2.99       2        5.98
Pepsi-Cola                    0.97       4        3.88
Fried Chicken                 3.99       3       11.97
SunChips                      2.59       3        7.77
Large Steak                  19.99       3       59.97
Canned Peas                   0.98       4        3.92
Swai Fish                     8.99       3       26.97
Catfish Fillets               9.99       2       19.98
Chicken Nuggets               2.95       5       14.75
Hershey Bar                   0.79      12        9.48
                                              ========
TOTAL                                           164.67
Press any key to continue . . .
*/

Then why does the original ReadFromFile function crash when attempting to read from this text file? It should work...

[jlb]

Look at this snippet:
while (getline(inFile, grocery[indexTotal].name, '#') && indexTotal < size)
You have your checks backward, you should be checking to insure indexTotal is < size before you try to use indexTotal as your array subscript.
while ( indexTotal < size && getline(inFile, grocery[indexTotal].name, '#'))

Next look at this snippet:

#include "GrItem.h"
// function prototype
void PrintReceipt(GrItem[], int);

int main()

Where are your #includes of the standard include files? You should never rely on pre-processor "magic" to #include required #include files. Always #include all the required #include files in every file where they're required. Also your header file doesn't need most of those #includes, it should only have the #includes that are actually required for that file. And using the "using" statement in the global scope of a header is a very bad practice since it defeats the purpose using namespaces. You should use the scope resolution operator to properly scope your namespaces in header files, "std::string", etc.

[MisterTams]

jlb,

I'll try out your code in a little bit here. I'll see if that works.

As for using the #include pre-processor directives I was taught that for efficiency sake you don't have to keep putting them into each and every file as long as you have them defined in a header file. So that you can just simply use the #include "ClassDefinition.h" to include all the other #include pre-processor directives in any other file.

Is it really that bad to do?

[MisterTams]

jlb,

The code you provided does solve the problem.

Still waiting on your response...

[jlb]

As for using the #include pre-processor directives I was taught that for efficiency sake you don't have to keep putting them into each and every file as long as you have them defined in a header file.

Then you were taught wrong. You should only include the include files that are required for that particular file.

For example your include file only needs the <string> header file, and nothing more. When you add a bunch of "extra" includes those "extra" includes are always included when you include that header file which leads to extra compile time. As an example your class source file also only needs the <string> header but because you've included all of those extra headers in your header they will also be included in your class source file, which will increase compile times.

Is it really that bad to do?

Yes.

[Thomas1965]

@jlb,

does it mean you don't like the idea of pre-compiled headers ?
They work only when you have one header that includes all the other headers that rarely change.
In theory that should actually speed up the build process.

[jlb]

does it mean you don't like the idea of pre-compiled headers ?

Pre-compiled headers are a different topic, but when used correctly they can be helpful when working with large projects. However, IMO, they are a waste of time for small projects such as this. The time to properly setup the pre-compiled header isn't worth the effort, IMO, for small project such as this.

Also you're not using pre-compiled headers so this is really not an issue for this program.