"Segmentation fault (core dumped)" Error

[PiggiesGoSqueal]

--------------------------------------------
Please check the most recent comments for the latest questions. Thanks!
--------------------------------------------

Please Note: The following program is a homework assignment, however it was due roughly 1-2 months ago. I'm doing it for the learning aspect of it.

I'm trying to write a program that will read from a file, do some stuff, and write to a file with the new info. My Algorithm Steps will explain more in detail (and what I've completed so far).

When I try to compile the code I get no errors but when I try to run it I get:
Segmentation fault (core dumped)
If I remember correctly, this can be caused by a for loop going out of an container's range for example (I think..?). But the for loop I have worked fine with v2 of my code so I don't think that's it.

Where the error is:
I commented out calling the functions in main() until it fixed the issue. Based off that test, the error appears to be somewhere in the readPData(...) function.

What am I doing wrong?

/*
Program Name: nba_v3.cpp
Date: 3/13/2019
Version: 3
Author: PiggiesGoSqueal
  
Status: Work in progress

Program Information:
  - A C++ Command Line Interface (CLI)-based menu-driven program that
	helps a client keep track of NBA players’ stats with certain
	requirements.


Algorithm Steps: (Note: Not tested since changing a lot of the code from v2 because of errors)
  1. Create a STRUCT that allows storing information for players. (DONE)
	 Must be able to store info for:
	 - player’s name, team name, points, rebounds, and assists (DONE)
	 - Be sure it is set up in such a way that allows easily storing (DONE)
	   a TON of players.
  2. Using a function, the program should read the existing players'
	 stats from the players.txt file. (DONE)
  3. Program should store data from players.txt into the struct. (DONE)
  4. Program should be menu driven, giving users various choices. (WORK IN PROGRESS)
	- Program must use a function to display players’ stats in a
	  cleanly formatted tabular form.   | text | text |
										---------------
										|      |      |
										---------------
	- Use function(s) to search the array to find the index of a
	  specific player, and update the data of an existing player.
	- Use a function to add a new player’s stat
  5. Use a function to sort the players’ stats based on player’s (DONE)
	 name and display the sorted array of records
  6. Before the program terminates, provide an option to save all the data (Not started)
     from memory back into the same input file; i.e. update the database.


Changes between v2 and v3:
- Rewriting and Optimizing some code
  * Removed count / counted variables (used for array)
  * Removed MAX (for array)
  * Edited general code
  * Switched array to vector
  * Switched bubblesort function to sortData() function (uses sort() algorithm)
- Removed some notes
*/
#include <iostream>
#include <string>
#include <vector>
#include <fstream> // for file reading and writing
#include <iomanip> // for formatting
#include <algorithm> // for sort()

using namespace std;


struct playerData {
	string pName; // player name
	string tName; // team name
	double points;
	double rebounds;
	double assists;
};

// Is it better to use references (&) or not for the vector of type struct?

// Reading player data from players.txt & storing it into vector of type playerData (struct)
void readPData(ifstream& playersFile, vector<playerData>& playerList);
// writes player data into a new output file for Debug purposes.
// Later I will need to make it update players.txt (the input file) directly.
void writePData(ofstream& playersFile, const vector<playerData> playerList);
// Sorts player stats based on player name (alphabetical order)
// Will also display the sorted vector of records
void sortData(vector<playerData>& playerList);
// Prints a menu & allows user to select some options (work in progress).
void printMenu(const vector<playerData>& playerList);


int main() {
	// Declaring player list vector which contains all player "objects"
	// for the struct.
    vector<playerData> pList;
	
	// Opening players.txt file:
	ifstream pFile; // input file
	string pFileName = "players.txt";
	pFile.open(pFileName);
	
    // If input file failed to open:
	if (!pFile)
		cerr << "File could not be opened.\n";

	// reads pFile data and stores information using pList[i].<struct_stuff>
	readPData(pFile, pList);

	// sorts player data based on player name (alphabetical order)
	// Will also display the sorted array of records
	sortData(pList);

    // Prints a menu & allows user to select some options (work in progress).
    printMenu(pList);

    // ---
    // writes player data into a new output file for Debug purposes.
    // Later I will need to make it update players.txt (the input file) directly.
	ofstream pFileOut;
    string pFileOutName = "playersOut.txt";
	pFileOut.open(pFileOutName);

	if (!pFileOut)
		cerr << pFileOutName << " could not be opened.\n";

	// will write output player data to a new file.
	writePData(pFileOut, pList);

    pFileOut.close();
    // ---

    // Closes input file.
	pFile.close();
	
	cout << "[Debug] Done!\n";

	return 0;
}


// Reads contents of the input file and stores contents into vector of type playerData (struct)
void readPData(ifstream& playersFile, vector<playerData>& playerList) {
	// While players file has not reached the end of file:
	for (int i = 0; getline(playersFile, playerList[i].pName); i++) {
		// playerList[i] is a player in the list. And playerList[i] has it's own parts (such as pName, tName, etc).
		// This for loop will go through all players in the file and store their info into the array.

		// I getline() for pName in the for loop, thus I don't add it here.
		getline(playersFile, playerList[i].tName); // this is for team name
		playersFile >> playerList[i].points;
		playersFile >> playerList[i].rebounds;
		playersFile >> playerList[i].assists;
		playersFile.ignore(1000, '\n'); // used because the >> adds a '\n' after it I think? For example, doing cin >> ... then doing getline() would have getline() be blank because of the '\n' before it.
		
		// Debug Msgs:
		/*
		std::cout << playerList[i].pName << "\n";
		std::cout << playerList[i].tName << "\n";
		std::cout << playerList[i].points << "\n";
		std::cout << playerList[i].rebounds << "\n";
		std::cout << playerList[i].assists << "\n\n\n";
		*/	
	}
}


// writes player data into a new output file for Debug purposes.
// Later I will need to make it update players.txt (the input file) directly.
void writePData(ofstream& oFile, const vector<playerData> playerList) {
    // Blank for now. Will add later.
}


// Should sort the player's stats based on the player's name.
// Should display the sorted vector.
void sortData(vector<playerData>& playerList) {
    
    for (int i = 0; i < playerList.size(); i++) {
        // should sort all player names in vector. If it does, I'll NEED TO ADD TNAME, POINTS, ETC.
        sort(playerList[i].pName.begin(), playerList[i].pName.end());

        // prints sorted vector contents:
        cout << playerList[i].pName << "   "
             << playerList[i].tName << "   "
             << playerList[i].points << "   "
             << playerList[i].rebounds << "   "
             << playerList[i].assists << "\n\n";
    }
}


// Prints a menu.
// Allows user to select some options.
void printMenu(const vector<playerData>& playerList) {
    int spaces = 3;
    int lengthM = 69;
	int pNameSize, tNameSize;
    // Prints Tabs:
    // |   player name   |   team name   |   points   |   rebounds   |   assists   |
    cout << setfill(' ')
         << "|" << setw(spaces+10) << "player name" << setw(spaces)
         << "|" << setw(spaces+8) << "team name" << setw(spaces)
         << "|" << setw(spaces+5) << "points" << setw(spaces)
         << "|" << setw(spaces+7) << "rebounds" << setw(spaces)
         << "|" << setw(spaces+6) << "assists" << setw(spaces)
         << "|" << "\n"
         << setfill('-') << setw(lengthM) << '\n'; 

    // Prints actual data:
    for (int i = 0; i < playerList.size(); i++) {
		pNameSize = playerList[i].pName.size();
		tNameSize = playerList[i].tName.size();
		//cout << "playerList[i].pName = " << playerList[i].pName << '\n';
		
		// NEED TO FIX (WORK IN PROGRESS) - Not sure why it's not working.
        cout << setfill(' ')
        << "|" << setw(spaces+pNameSize) << playerList[i].pName << setw(spaces)
        << "|" << setw(spaces+tNameSize) << playerList[i].tName << setw(spaces);
		
		cout << "\n";
    }

	cout << "[Debug] printMenu() done! \n";
}

Help would be greatly appreciated. Thank you!

[PiggiesGoSqueal]

Sorry if the indenting is a bit off, whenever I try to copy & paste from Visual Studio Code it causes some indenting issues. If anyone knows how to fix that, feel free to tell me that as well! :)

[Repeater]

vector<playerData> pList;
This creates a vector of size zero. It can hold zero elements. If you try to read or write to any element, there's risk of a crash. It's bad.

readPData(pFile, pList);
Pass that size zero vector to a function...

for (int i = 0; getline(playersFile, playerList[i].pName); i++) {
Start trying to write data into non-existent elements of that size zero vector.


You're trying to write to elements of the vector that don't exist.

If I remember correctly, this can be caused by a for loop going out of an container's range for example (I think..?). But the for loop I have worked fine with v2 of my code so I don't think that's it.

That's it. The container is of size zero, containing no elements, and you're trying to write to those non-existent elements.

[dhayden]

To fix the problem that Repeater found, read into a dummy instance and then push the dummy onto the vector:

void readPData(ifstream& playersFile, vector<playerData>& playerList) {
	playerData dummy;
	// While players file has not reached the end of file:
	for (int i = 0; getline(playersFile, dummy.pName); i++) {
		getline(playersFile, dummy.tName); // this is for team name
		playersFile >> dummy.points;
		playersFile >> dummy.rebounds;
		playersFile >> dummy.assists;
		playersFile.ignore(1000, '\n');
		playerList.push_back(dummy);

		// Debug Msgs:
		/*
		std::cout << playerList[i].pName << "\n";
		std::cout << playerList[i].tName << "\n";
		std::cout << playerList[i].points << "\n";
		std::cout << playerList[i].rebounds << "\n";
		std::cout << playerList[i].assists << "\n\n\n";
		*/	
	}
}

[PiggiesGoSqueal]

Ah okay, thank you for the reply @Repeater.

vector<playerData> pList;
This creates a vector of size zero. It can hold zero elements. If you try to read or write to any element, there's risk of a crash. It's bad.

To clarify, I should never declare an empty vector? Even if I will push elements to it?

Also, I changed the vector declaration line to:
vector<playerData> pList(100);

Then I was going to use playerList.shrink_to_fit() after the loop but after checking:
- http://www.cplusplus.com/reference/vector/vector/shrink_to_fit/
- http://www.cplusplus.com/reference/vector/vector/size/
I realized that playerList.size() should return the number of actual objects in the vector. Meaning that other functions where playerList.size() is used should still work as intended.

However, when I actually tested it, playerList.size() = 100. So how can I make it so my for loops in other functions will only go run for the size of the actual objects in the vector? I switched from array to vector so I wouldn't need to have a counter variable.

-----------------------------
Also, I just realized that my sortData() function won't work I believe because my goal is to sort the player data based off their names. For example:

players.txt

Hatz
team name
5
3
2
Hatx
team name
4
6
9
Haty
team name
1
5
3
Hata
team name
8
6
7

The program should check the playerList.pName's and sort them based off alphabetical order and each player name should keep the player data too (team name, points, rebounds, assists) so each player name still has the correct player data attached to it.

Is this possible to do with the sort() algorithm? Or do I need to switch back to bubble sort?

Thanks!

[Repeater]

To clarify, I should never declare an empty vector?

You should never try to read from or write to an element of a vector that doesn't exist. There's nothing wrong with having a size zero container.

Even if I will push elements to it?

When you push_back an element to a vector, the vector grows in size by one.

vector<int> someVector;// size zero. No elements
someVector.push_back(1); // Now the vector is size one. It has one element. 

vector<int> someOtherVector(100);// 100 elements in this vector
someOtherVector.push_back(1); // Now the vector has one hundred and one elements. It is bigger than it was 

So how can I make it so my for loops in other functions will only go run for the size of the actual objects in the vector?

Every object in a vector is an actual object.
vector<int> someOtherVector(100);// There are 100 ACTUAL objects in this vector

[PiggiesGoSqueal]

Thanks for your reply, very helpful. Sorry for late reply.

Every object in a vector is an actual object.
vector<int> someOtherVector(100);// There are 100 ACTUAL objects in this vector

I'm confused. Okay so let's say I've created a string vector of 100 objects. Then push_back("hello").

That "hello" would go into the first vector position (position 0) because nothing has been assigned to that spot right? And the total vector size would become 101.

Also, let's say I push_back("world") after that. How can I make the vector run through the objects I added and not the empty positions?

Thanks!

[Repeater]

That "hello" would go into the first vector position (position 0) because nothing has been assigned to that spot right? And the total vector size would become 101.

No. If you have a vector of 100 objects, and you push_back, the object you pushed goes on the end.

push_back means "create a whole new object on the end of the vector, and that object is to have this value".

[E0p9LyTq]

Okay so let's say I've created a string vector of 100 objects. Then push_back("hello").

Your string vector now contains 101 objects, with your pushed back "hello" object at the end.

The 101st element (someVector[100])

I push_back("world") after that

Now 102 objects in your vector.

How can I make the vector run through the objects I added and not the empty positions?

Don't create a sized vector. Create an empty vector, push back all the objects you want to work with.

Now you can loop through your vector using either:

1. the size of the vector for (unsigned i = 0; i < someVector.size(); i++

2. std::vector's iterator methods for (auto itr = someVector.begin(); itr != someVector.end(); itr++)

3. a range-based loop for (auto& itr : someVector)

Each for loop will cycle through your vector without you needing to manually keep track of the number of elements.

Which one you use depends on what you want/need to do with the elements.

There are variations of the loops using const that prevent modifying the elements. If I was looping through the vector to printout the elements I might use for (const auto& itr : someVector).

[E0p9LyTq]

Creating a sized vector is IMO only useful when you know how many data elements you will be working with from the start.

For instance, if you have a data file with 100 lines of text std::vector<std::string> data(100); might be a good idea. Then you can read each line in your file and store them in your vector using operator[] or .at() in a loop.

Or if the user specifies the number of elements for, say, a game board.

When you don't know how many elements, create an empty vector and push back.

Even a sized-on-creation vector can be expanded with push_back.

You can have a sized vector, push back some elements and deal only with the pushed back elements. The amount of overhead to track that makes creating an empty vector easier.

[dhayden]

Building on what FurryGuy said, there are also efficiency concerns. If you preallocate 100 items and then assign to them, you're executing the constructor 100 times and then overwriting the values when you assign to them. If the constructor is complicated, this could affect performance.

If you know about how many items you'll need and if the constructor is complex and if you're setting them one at a time from the beginning, then it may be better to allocate an empty vector and then use string::reserve() to reserve space for the anticipated number of items. Reserve(n) tries to ensure space for n items but doesn't construct anything in the extra space.

[PiggiesGoSqueal]

Ahh okay, thank you Repeater, FurryGuy, and dhayden. All very informative and helpful responses. I now understand the question I asked, in addition to, when I should use an empty vector vs when I shouldn't.

Also, thanks dhayden for your response about the dummy instance, I didn't see it before but I noticed it now.

[PiggiesGoSqueal]

Okay, I have two issues left. First I can't get my printMenu() function to print the player names and team names. It only prints the points, rebounds, and assists. I'm thoroughly confused because printing the name alone works (example is commented out in code at line 174:

//cout << "[Debug] " << playerList[i].pName << endl; 

However, when I try to print it in the menu (with or without a template function) then it doesn't work. You can view the printMenu() function and template function from line 140 and further.

The second issue is I can't get the sortData() function to work like I want. My post:
http://www.cplusplus.com/forum/beginner/251018/#msg1105413 explains what I was trying to do. Will I need to use bubblesort instead?

Latest Code:

/*
Kattis Program: nba.cpp
Date: 3/19/2019
Version: 3
Author: PiggiesGoSqueal
  
Status: Work in progress

Program Information:
  - A C++ Command Line Interface (CLI)-based menu-driven program that
	helps a client keep track of NBA players’ stats with certain
	requirements.

Algorithm Steps:
  1. Create a STRUCT that allows storing information for players. (DONE)
	 Must be able to store info for:
	 - player’s name, team name, points, rebounds, and assists (DONE)
	 - Be sure it is set up in such a way that allows easily storing (DONE)
	   a TON of players.
  2. Using a function, the program should read the existing players'
	 stats from the players.txt file. (DONE)
  3. Program should store data from players.txt into the struct. (DONE)
  4. Program should be menu driven, giving users various choices. (Work in progress)
	- Program must use a function to display players’ stats in a
	  cleanly formatted tabular form.   | text | text |
										---------------
										|      |      |
										---------------
  5. Use function(s) to search the vector to find the index of a
	  specific player, and update the data of an existing player.
  6. Use a function to add a new player’s stat.
  7. Use a function to sort the players’ stats based on player’s name and
     display the sorted array of records.
     * Tried doing with sort() algorithm. Doesn't seem to work? So may need to switch back to bubblesort.
*/
#include <iostream>
#include <string>
#include <vector>
#include <fstream> // for file reading and writing
#include <iomanip> // for formatting
#include <algorithm> // for sort()

using namespace std;


struct playerData {
	string pName; // player name
	string tName; // team name
	double points;
	double rebounds;
	double assists;
};


// Function Declarations:
// Reading player data from players.txt & storing it:
// Using references (&) to directly update info without returning anything.
// Remember: pointers (*) point directly to the ADDRESS of a variable, not the variable itself.
void readPData(ifstream& playersFile, vector<playerData>& playerList);
// Sorts player stats based on player name (alphabetical order)
// Will also display the sorted vector of records
void sortData(vector<playerData>& playerList);
// Prints a menu.
// Allows user to select some options.
void printMenu(const vector<playerData>& playerList);

int main() {
	// Declaring player list vector which contains all player "objects"
	// for the struct.
    vector<playerData> pList;
	
	// Opening players.txt file:
	ifstream pFile; // input file
	string pFileName = "players.txt";
	pFile.open(pFileName);
	
	if (!pFile)
		cerr << "File could not be opened.\n";

	// reads pFile data and stores information using pList[i].<struct_stuff>
	readPData(pFile, pList);

    printMenu(pList);

	// sorts player data based on player name (alphabetical order)
	// Will also display the sorted array of records
	//sortData(pList);

	pFile.close();
	
	cout << "[Debug] Done!\n";

	return 0;
}

/*
  The following function will:
  - Read the contents of the file and assign it's contents into
	playerList[i].pname, playerList[i].tname, etc.
  - Because vector<playerData>& playerList is using a reference, it should auto
	update everything for that without needing to return anything for
	the function.
*/
void readPData(ifstream& playersFile, vector<playerData>& playerList) {
	playerData temp;
	// While players file has not reached the end of file:
	for (int i = 0; getline(playersFile, temp.pName); i++) {
		// playerList[i] is a player in the list. And playerList[i] has it's own parts (such as pName, tName, etc).
		// This for loop will go through all players in the file and store their info into the array.

		// I getline() for pName in the for loop, thus I don't add it here.
		getline(playersFile, temp.tName); // this is for team name
		playersFile >> temp.points;
		playersFile >> temp.rebounds;
		playersFile >> temp.assists;
		playersFile.ignore(1000, '\n');
		playerList.push_back(temp); // adds all parts above to playerList vector (of type playerData) all at once.
	}
}


// This function should sort the player's stats based on the player's name (alphabetical order). - Doesn't work at the moment
// Then should display the sorted array of records. (This part isn't started yet)
void sortData(vector<playerData>& playerList) {
    for (int i = 0; i < playerList.size(); i++) {
        // should sort all player names in vector. If it does, I'll NEED TO ADD TNAME, POINTS, ETC.
        sort(playerList[i].pName.begin(), playerList[i].pName.end());

        // prints sorted vector contents:
        cout << playerList[i].pName << "   "
             << playerList[i].tName << "   "
             << playerList[i].points << "   "
             << playerList[i].rebounds << "   "
             << playerList[i].assists << "   \n";
    }
    cout << "[Debug] sortData() function done!\n";
}


// Used in printMenu(), allows easily printing stuff in menu.
template<typename T> void printElem(T t, const int& width) {
	cout << left << setw(width) << setfill(' ') << t;
}


/*
  4. Program should be menu driven, giving users various choices.
	- Program must use a function to display players’ stats in a
	  cleanly formatted tabular form.   | text | text |
										---------------
										  text   text            
										  text   text
      * Tabs: player name | team name | points | rebounds | assists |
*/
void printMenu(const vector<playerData>& playerList) {
    const int spaces = 4;
	const int nameWidth = 6;
	const int numWidth = 8;
	const int lengthM = 69; // length of menu (used for dashes)

	// Prints Tabs:
    // |   player name   |   team name   |   points   |   rebounds   |   assists   |
    cout << setfill(' ')
         << setw(spaces+10) << "player name" << setw(spaces)
         << setw(spaces+8) << "team name" << setw(spaces)
         << setw(spaces+5) << "points" << setw(spaces)
         << setw(spaces+7) << "rebounds" << setw(spaces)
         << setw(spaces+6) << "assists" << setw(spaces)
         << "\n"
         << setfill('-') << setw(lengthM) << '\n'; 

    // Prints actual data:
    for (int i = 0; i < playerList.size(); i++) {
		//cout << "[Debug] " << playerList[i].pName << endl;
		printElem(playerList[i].pName, nameWidth);
		printElem(playerList[i].tName, nameWidth);
		printElem(playerList[i].points, numWidth);
		printElem(playerList[i].rebounds, numWidth);
		printElem(playerList[i].assists, numWidth);
		cout << endl;
    }

	cout << "[Debug] printMenu() done! \n";
}

players.txt:

Hatz
team name
5
3
2
Hatx
team name
4
6
9
Haty
team name
1
5
3
Hata
team name
8
6
7

Code Output: (Wouldn't work properly when I tried pasting it in code tags)

player name team name points rebounds assists
--------------------------------------------------------------------
5 3 2
4 6 9
1 5 3
8 6 7
[Debug] printMenu() done!
[Debug] Done!


Help would be greatly appreciated, thank you!

[E0p9LyTq]

no one responded to

Could be no one noticed, since you have two threads with much the same title.

Begging/bumping threads generally won't encourage people to respond.

[PiggiesGoSqueal]

? I wasn't begging or bumping to get people to respond? I apologize if it came off that way, that definitely was not my intention. I am fully aware that neither of those would produce responses.

Edit: Also, I edited my previous message, you probably responded before I finished editing / posting it.

[dhayden]

Line 127 sorts player[i]'s . So the name "dave" becomes "adev". The loop at 125 ensures that you do this to each name.

The easiest way to sort the names is to start by defining a less-than operator on struct playerData that puts them in the order you want. In this case, it's by pName:

struct playerData {
	string pName; // player name
	string tName; // team name
	double points;
	double rebounds;
	double assists;
	bool operator<(const playerData &rhs) const { return pName < rhs.pName); }
};

Now sortData() becomes a one-liner:

void sortData(vector<playerData>& playerList) {
	std::sort(playerList.begin(), playerList.end());
}

You can add a loop after the call to sort() to print them out.

[PiggiesGoSqueal]

Line 127 sorts player[i]'s . So the name "dave" becomes "adev". The loop at 125 ensures that you do this to each name.

Ah haha, that makes sense dhayden. Thanks for your response!

The easiest way to sort the names is to start by defining a less-than operator on struct playerData that puts them in the order you want. In this case, it's by pName:

struct playerData {
	string pName; // player name
	string tName; // team name
	double points;
	double rebounds;
	double assists;
	bool operator<(const playerData &rhs) const { return pName < rhs.pName); }
};

Hmm I don't recall learning about code formatted like that. Could you explain what is happening, or link me to something that explains it?
I briefly checked this: https://en.cppreference.com/w/cpp/language/operator_comparison
and it looks similar but I don't really understand. Maybe I just need to read more of it, I don't know.

[dhayden]

Your link is good. I'm defining operator<() using inline coding. Does this make more sense?

struct playerData {
	string pName; // player name
	string tName; // team name
	double points;
	double rebounds;
	double assists;
	bool operator<(const playerData &rhs) const;
}

bool playerData::operator<(const playerData &rhs) const
{
    return (pName < rhs.pName);
};

[nuderobmonkey]

Another possibility is, overloading the < operator by a freestanding function:

bool operator< (const playerData & lhs, const playerData & rhs)
{
    return lhs.pName < rhs.pName;
}

[PiggiesGoSqueal]

Thank you both for responding!

Yes, @dhayden having it on multiple lines helps me understand more but I also meant I've never seen "operator<".

I'm still pretty new to overloading and I don't see how sort() knows to check the bool operator< function then sort them based off that.

Does sort() use < so it is checking the struct object contents and finds/uses the < operator function to determine which is smaller / larger? Am I making sense? Sorry..