Sorting strings by alphabetical order (in an array using a struct)

[PiggiesGoSqueal]

Please Note: I did post a question already for this assignment, however, this is a new question and my code has changed a lot since the original post so I felt it would be more clear if I just made a new post. The old post is marked as solved as well, as that issue was solved (http://www.cplusplus.com/forum/beginner/250211/).

I was assigned a problem for class, and even though the due date was ~1 month ago, I would still like to do it for the learning aspect of it. Anyways, my program is basically meant to:
- read from a file
- store the information into an array using a structure
- sort the information in alphabetical order (based on the player names)
- perform some other features
- then update the text file that was storing the information.

I've only gotten as far as trying to sort the information in alphabetical at the moment.

For the alphabetical sorting, I'm trying to use a bubble sort function. But it's not working like I want. I compare the letters in the names by using their ASCII values and if Letter 1 value > Letter 2 value it should switch them. However, if they equal each other, it should compare the 2nd letters for each name. And if those equal each other, it should compare the 3rd letter for each name... and so on... (until one of the names has hit the end).

Please Note: If there's an easier way please let me know. Although I still struggle understanding pointers and references.

I'm kinda hesitant to copy and paste my full code because it's 157 lines long (with comments of what's going on & debug msgs).. But I also think it'd be pretty difficult to read only a snippet without seeing the structure setup etc.. So.. I guess I'll just post the full code? Let me know if I should only be posting a certain section.

#include <iostream>
#include <string>
#include <fstream> // for file reading and writing

// to reduce number of errors I may get. Can remove this when done and retest.
using namespace std;


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


// Function Declarations:
// Reading player data from players.txt & storing it:
void readPData(std::ifstream& playersFile, playerData* playerList, const int maxPlayers, int& counter); 
// Sorts player stats based on player name (alphabetical order)
// Will also display the sorted array of records
void sortStats(playerData* playerList, const int maxPlayers, int& counter);
// bubble sort is an add-on function for sortStats();
void bubbleSort(playerData* playerList, const int maxPlayers, int& counted);

int main() {
	// Declaring player list array which contains all player "objects"
	// for the struct.
	const int PMAX = 100;
	playerData pList[PMAX];
	int count = 0; // will be used in readPData & bubbleSort()

	
	// Opening players.txt file:
	std::ifstream pFile; // input file
	std::string pFileName = "players.txt";
	pFile.open(pFileName);
	
	if (!pFile)
		std::cerr << "File could not be opened.\n";

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

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

	std::cout << "[Debug] Done!\n";

	return 0;
}

/*
  The following function will:
  - Read the contents of the file and assign it's contents into
	playersList[i].pname, playersList[i].tname, etc.
  - Because playerData* playerList is using a pointer, it should auto
	update everything for that without needing to return anything for
	the function.
*/
void readPData(std::ifstream& playersFile, playerData* playerList, const int maxPlayers, int& counter) {
	// While players file has not reached the end of file:
	for (int i = 0; playersFile.eof() == false; 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.

		
		std::getline(playersFile, playerList[i].pName); // will check file line and assign the line contents to playerList[i].pName
		std::getline(playersFile, playerList[i].tName);
		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.
		/*
		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";
		*/
		counter++; // updates count in main() which will be used in bubbleSort() too.
	}
}


/*
  This function should sort the player's stats based on the player's name (alphabetical order).
  Then should display the sorted array of records. (This part isn't started yet)
*/
void bubbleSort(playerData* playerList, const int maxPlayers, int& counted) {
	// while i < counted (the # number of names stored):
	for (int i = 0; i < counted; i++) {
		bool actionTakenThisRound = false;
		int letterPos = 0;
		
		/*
		  Notes for the Following Part:
		  - Example of what it's doing:
		    if ("Z" > "A") { switch Z & A; Then set actionTakenThisRound to true; }
			* Specifically, it's comparing object 1's first character in the player name with object 2's first char in the player name.
		  - When it comes to the order of the alphabet, lower ASCII values
		    means it's closer to the beginning of the alphabet.
			* So, 'a' would have the lowest ASCII value for all lowercase letters.
			* In other words, lower values should be first and greater values last.
		*/
		if (playerList[i].pName[i] > playerList[i+1].pName[i]) {
			playerList[i].pName.swap(playerList[i+1].pName); // swaps the 2
			actionTakenThisRound = true;
		}

		/*
		  Notes for this section:
		  - This checks if the first letter in each player name matches the other player name's first letter. If so,
		    it will set up a loop that will compare the 2nd letters against each other. If they're different, it will
			switch if needed. Else it will compare the 3rd letters, etc... Until it comes across letters that are different
			or until one of the player names runs out of letters to check.
		*/
		else if (playerList[i].pName[0] == playerList[i+1].pName[0]) {
			// declares the sizes of the names (used in for loop below)
			int pName0Size = playerList[i].pName.size()-1;
			int pName1Size = playerList[i+1].pName.size()-1;

			for (; ( (letterPos < pName0Size) && (letterPos < pName1Size) ); letterPos++ ) {
				std::cout << "[Debug] letterPos = " << letterPos << '\n'
						  << "[Debug] pName0Size = " << pName0Size << '\n';

				if (playerList[i].pName[letterPos] > playerList[i+1].pName[letterPos]) {
					playerList[i].pName.swap(playerList[i+1].pName);
					break;
				}
			}
			/*
			std::cout << "[Debug Info Below]\n"
					  << "i = " << i << '\n'
					  << "letterPos = " << letterPos << '\n'
					  << "pName0Size = " << pName0Size << '\n'
					  << "pName_i1_Size = " << pName_i1_Size << '\n'
					  << "playerList[i].pName = " << playerList[i].pName << '\n'
					  << "playerList[i+1].pName = " << playerList[i+1].pName << '\n';
			*/

			// More Debug Info
			std::cout << "playerList[0].pName = " << playerList[0].pName << '\n';
			std::cout << "playerList[1].pName = " << playerList[1].pName << '\n';
			std::cout << "playerList[2].pName = " << playerList[2].pName << '\n';
			std::cout << "playerList[3].pName = " << playerList[3].pName << '\n';

			actionTakenThisRound = true;
		}

	    if (actionTakenThisRound == true) // not sure if I should reduce i or letterPos or neither.
			//i--;
			letterPos--;
	}
}

Help would be greatly appreciated. Thanks! :)

[Handy Andy]

Hello PiggiesGoSqueal,

While I look at your program would you please post the input file or at least a fair sample ( 5 records or so).

Andy

[PiggiesGoSqueal]

Oops I forgot about that!

Here's the example I've been testing with that helps me check if it will go through each matching letters until it finds a difference then sorts accordingly:

Hatz
team name
3
2
1
Hatx
team name
1
2
3
Haty
team name
1
2
3
Hata
team name
1
2
3

It should sort in the order: Hata, Hatx, Haty, Hatz

Then here's a normal file:

Stephen Curry
Golden State Warriors
29.8
5.2
6.5
Lebron James
Cleveland Cavaliers
25.7
7.4
6

[E0p9LyTq]

Based on your input file (second), are you sorting on a player's last name or the entire string? That can make a difference.

Sorting on last name might require storing the player's name as separate strings: first, middle (if any) and last name.

If you go to that level of complexity tokenizing out the full team name into city/region and team name would be easy to achieve.

[dhayden]

PiggiesGoSqueal, that's an excellent original post. You explained the problem, the context (assignment that was already due) and posted the full program, which is really helpful so we can compile it. Nice job.

sortStats() and bubbleSort() should pass counter by value since you don't change it.

You need a writePData() function to write the data out. This would really help because you could test the read/write functions before testing bubbleSort(). Otherwise, if the data is wrong you could spend hours looking for a bug in bubbleSort() when the real problem is in readPData().

Line 65. stream::eof() won't become true until you actually try to read PAST the end of file. So right now, if will be false after reading the last record and you'll attempt to read another one. To fix this, attemptto read the player name in the condition:

    // 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.

        std::getline(playersFile, playerList[i].tName);
        ...

Using getline() as the condition may need some explanation. Here's how it works:
- getline returns the stream(playersFile).
- The for() statement wants a bool, so the compiler looks to a way to convert an istream to a bool.
- istream has bool() operator defined. It simply calls istream::fail().

So the net result is that you call getline() to read the line, and then call playersFile.fail() to see if the stream is still valid. Pretty cool!

Line 94: since you're comparing playerList[i].pName to playerList[i+1].pName, the loop needs to end at counted-1, not counted:

The rest of bubbleSort is kind of a mess I'm afraid. Inside the loop you need to do two things: (1), compare the two records (playerList[i] to playerList[i+1], and then (2) if necessary, swap them. Right now you're kind of mixing it all up. Just concentrate on writing the comparison code. Then the swap code is pretty easy.

For starters, just compare the pName strings directly.

Then you can use std::swap() to swap the whole record.

You also need TWO loops in bubbleSort(). The outer loop runs until you don't swap any items. The inner loop makes one pass through the data, swapping items that are out of order:

    bool actionTakenThisRound;
    do {
        actionTakenThisRound = false;

        // Make a pass through the array. If you swap anything then remember it.
        // [ CODE NOT SHOWN ]
    } while (actionTakenThisRound); // keep looping until you make a pass without swapping 

[chicofeo]

Out of many and better ways to do it, below is to give you an idea.

The players.txt file:

Stephen Curry
Golden State Warriors
29.8
7.6
6.5
Lebron James
Cleveland Cavaliers
25.7
7.4
6.2
Nukola Jovic
Denver Nuggets
35.6
11.4
11.8
Anthony Davis
New Orleans Pelicans
36.2
19.1
8.4
James Harden
Houston Rockets
61.3
15.6
4.7

#include <iostream>
#include <string>
#include <fstream>
#include <limits> // For std::numeric_limits

struct playerData
{
	std::string pName;
	std::string tName;
	double points;
	double rebounds;
	double assists;
};

int countRecords(std::ifstream&, std::string, bool&);
void readFile(std::ifstream&, playerData*, std::string, int);
void sort(playerData*, int);
void writeFile(std::ofstream&, playerData*, std::string, int);

int main()
{
	std::ifstream inFile;
	std::string inFileName{ "players.txt" };
	bool goodRead{}; // Used to set the flag for data validation in the file.
	int size = countRecords(inFile, inFileName, goodRead);

	std::ofstream outFile;
	std::string outFileName{ "sortedPlayers.txt" };
	
	if (goodRead) // if there are no data validation issues, then proceed.
	{
		std::cout << "There are " << size << " records in " << inFileName << std::endl;

		// Allocate the memory based on the count of records.
		playerData *pList = nullptr;
		pList = new playerData[size];

		readFile(inFile, pList, inFileName, size);
		sort(pList, size);
		writeFile(outFile, pList, outFileName, size);

		delete [] pList;
		pList = nullptr;

	}

	else // Notify user of the first data validation fault is found.
	{
		std::cout << "Please, verify the validation data on " << inFileName << std::endl;
	}

}
/*
	This functions will open the reading file to validate the requested
	data validation and count the number of records. If there is a violation of the
	data validation, then the goodRead will be set to false and will notify the user
	of the first data violation found.
*/
int countRecords(std::ifstream& inFile, std::string inFileName, bool &goodRead)
{
	inFile.open(inFileName);
	playerData temp;
	goodRead = true;
	int counter = 0;

	if (!inFile)
	{
		std::cout << "Error opening " << inFileName << std::endl;
		goodRead = false;
	}

	else
	{

		while (std::getline(inFile, temp.pName) && goodRead)
		{
			std::getline(inFile, temp.tName);

			inFile >> temp.points;
			if (inFile.fail())
			{
				inFile.clear();
				inFile.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				std::cout << "Error reading points from "
					<< temp.pName << std::endl;
				goodRead = false;
			}

			inFile >> temp.rebounds;
			if (inFile.fail())
			{
				inFile.clear();
				inFile.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				std::cout << "Error reading rebounds from "
					<< temp.pName << std::endl;
				goodRead = false;
			}

			inFile >> temp.assists;
			if (inFile.fail())
			{
				inFile.clear();
				inFile.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
				std::cout << "Error reading assists from "
					<< temp.pName << std::endl;
				goodRead = false;
			}

			inFile.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
			counter++;
		}
		inFile.close();
	}

	return counter;
}

void readFile(std::ifstream& inFile, playerData* pList, std::string inFileName, int size)
{
	inFile.open(inFileName);

	if (!inFile)
	{
		std::cout << "Error opening " << inFileName << std::endl;
		return;
	}

	else
	{
		std::cout << "File " << inFileName << " open.\n";

		for (int count = 0; count < size; count++)
		{
			std::getline(inFile, pList[count].pName);
			std::getline(inFile, pList[count].tName);
			inFile >> pList[count].points;
			inFile >> pList[count].rebounds;
			inFile >> pList[count].assists;

			inFile.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
		}
		inFile.close();
		std::cout << "File " << inFileName << " closed.\n";
	}

}

void sort(playerData *pList, int size)
{
	bool swap{};
	playerData temp;

	std::cout << "Sorting the records by players first name...\n";

	do
	{
		swap = false;

		for (int count = 0; count < (size - 1); count++)
		{
			// Sorted by alphabetical name.
			// Then move the rest of the players
			// information with the sorted data.
			if (pList[count].pName > pList[count + 1].pName)
			{
				temp.pName = pList[count].pName;
				pList[count].pName = pList[count + 1].pName;
				pList[count + 1].pName = temp.pName;

				temp.tName = pList[count].tName;
				pList[count].tName = pList[count + 1].tName;
				pList[count + 1].tName = temp.tName;

				temp.points = pList[count].points;
				pList[count].points = pList[count + 1].points;
				pList[count + 1].points = temp.points;

				temp.rebounds = pList[count].rebounds;
				pList[count].rebounds = pList[count + 1].rebounds;
				pList[count + 1].rebounds = temp.rebounds;

				temp.assists = pList[count].assists;
				pList[count].assists = pList[count + 1].assists;
				pList[count + 1].assists = temp.assists;

				swap = true;
			}
		}

	} while (swap);

	std::cout << "Sorting completed.\n";

}

void writeFile(std::ofstream& outFile, playerData* pList, std::string outFileName, int size)
{
	outFile.open(outFileName);
	std::cout << "File " << outFileName << " open.\n";

	for (int count = 0; count < size; count++)
	{
		outFile << pList[count].pName << '\n'
			<< pList[count].tName << '\n'
			<< pList[count].points << '\n'
			<< pList[count].rebounds << '\n'
			<< pList[count].assists << '\n';
	}

	outFile.close();
	std::cout << "File " << outFileName << " closed.\n";
}

[PiggiesGoSqueal]

... Well, finally finished my response to @FurryGuy and @Dhayden (with quotes to make it easier to understand what I'm talking about). Then clicked submit. Told me it was too long and wiped my entire response. So.. Here's my shorter version:

Thank you everyone for replying! I very much appreciate it!

-----------------------------------------------------------------------------------
@FurryGuy, good question. I am sorting them alphabetically by the first letter, then second, and so on. Not by last name.

-----------------------------------------------------------------------------------
@dhayden, thank you for your very thorough response! It was incredibly helpful. Took me a bit to wrap my head around some parts, but had you only shown me code (without an explanation for example), I would've been incredibly lost. So I deeply appreciate the time you put into all of that, it was very helpful!

Edit: Fixed the problem I was having, so I removed the errors / code from this msg.

The issues this post was about has been fixed. Thank you all!

[PiggiesGoSqueal]

@chicofeo thank you for your response / example solution!

Unfortunately, I haven't learned many of the things you provided in your code yet. However, it did help me to see how you swapped the player data! Like I asked dhayden, is that the only way to go about switching them? swap() won't work at all?

Thanks!

[chicofeo]

@PiggiesGoSqueal,

The beauty of programming is you can solve the same problem by using different methods. I am not sure, but due to the vast capabilities C++, I do not think it is the only way (my opinion, at least).

[PiggiesGoSqueal]

Ah okay, yeah probably. Thanks