sort alogrithm with classes

[imohamme5]

Hi guys, I am attempting the following question:

A program is required to register user votes for a number of talent contest candidates. When all the votes have been cast, the program should sort the candidates into order, based on the number of votes they have received and output them to the screen.
Create a Candidate class to represent the candidate, storing their name and the number of votes they receive (initially 0). The class should provide methods to get and set the name, increment the votes by 1, and get the number of votes.
The voting program should prompt the user to enter the number of candidates, followed by that number of candidate names.
The program then prompts the user to repeatedly enter the names of the candidates; each input of a name is a vote and the program increments the number of votes for that candidate by 1.
When all voting has finished, the user should enter "end" and the program should sort the Candidate objects into descending order of number of votes and output the results.

I am having trouble with the sort function, I tried to use the bubble sort algorithm. I understand the code, and can implement it in a normal program without any classes. However I am a bit confused how it works with classes. Any help would be greatly appreciated, my attempts at coding it is below.

Main:

//Task 2 Votes
#include "candidates.h"
#include "Header.h"
#include <iostream>
#include <string>

using namespace std;

int main()
{
	int num = 0, j = 0;
	cout << "Enter the number of candidates: ";
	cin >> num;
	cin.ignore();
	candidates* c = new candidates[num];
	
	readNames(c, num, j);

	vote(c, num);
	
	sort(c, num);

	print(c, num);
	
	delete[] c;

	system("PAUSE");
	return 0;
}

void readNames(candidates c[], int num, int j)
{
	string Cname;
	for (int i = 0; i < num; i++)
	{
		j++;
		cout << "\nEnter the name of Candidate no. " << j << ": ";
		getline(cin, Cname);
		c[i].setName(Cname);
		cout << endl;
	}
	 
}

void vote(candidates c[], int num)
{
	string vote;

	cout << "\nType <end> to close the program";

	do
	{
		cout << "\nEnter the name of the candidate you woulld like to vote for: ";
		getline(cin, vote);
		for (int j = 0; j < num; j++)
		{
			if (vote == c[j].getName())
			{
				c[j].incrementVotes();
			}
		}

	} while (vote != "end");
}

void sort(candidates c[], int num)
{
	int j = 0;
	bool swap = true;
	
	string temp;
	while (swap)
	{
		swap = false;
		j++;
		for (int l = 0; l < num - j; l++)
		{
			if (c[l].getName() > c[l + 1].getName())
			{
				temp = c[l].getName();
				c[l].getName() = c[l + 1].getName();
				c[l + 1].getName() = temp;
				swap = true;
			}
		}
	}

}

void print(candidates c[], int num)
{
	for (int k = 0; k < num; k++)
	{
		cout << "\nno. of votes for " << c[k].getName() << ": " << c[k].incrementVotes() << endl;
	}
}

Header:

void readNames(candidates[], int, int);
void vote(candidates[], int);
void sort(candidates[], int);
void print(candidates[], int);

Candidates Class:

#include "candidates.h"

candidates::candidates()
{
	noOfvotes = 0;
}


candidates::~candidates()
{
}

void candidates::setName(string n)
{
	name = n;
}

string candidates::getName()
{
	return name;
}

int candidates::incrementVotes()
{
	
	return noOfvotes++;
}

Candidadates class header:

#pragma once
#include <string>
using namespace std; 

class candidates
{
public:
	candidates();
	~candidates();

	void setName(string);
	string getName();
	
	//void cVotes(string);

	int incrementVotes();
private:
	int noOfvotes;
	
	string name;
	string votes;
};

At the moment everything works as expected, besides the sort function, as it does not work.

[gunnerfunner]

Instead of string temp (line 71) you'd need:

(a) a candidates temp,
(b) overload copy assignment operator,
(c) following (b), definitions of copy ctor and dtor.

Then you'd do the bubble sort on the votes data-member just as you'd bubble sort an array of POD but instead of moving POD around here you'd be moving candidates objects around using the temp from (a) as a place-holder.

Alternatively you can save yourselves all these steps by using std::vector and then std::sort with a defined comparison function based on votes

[imohamme5]

Thanks for the reply, but could you word it in more simpler terms? I am new to c++ all I have understood so far from your response is to change string temp to candidates temp, which makes sense.

I don't understand the rest and haven't learnt about vectors yet.

I have made the following changes:

void sort(candidates c[], int num)
{
	int j = 0;
	bool swap = true;
	
	candidates temp;
	while (swap)
	{
		swap = false;
		j++;
		for (int l = 0; l < num - j; l++)
		{
			if (c[l].getName() > c[l + 1].getName())
			{
				temp = c[l];
				c[l] = c[l + 1] ;
				c[l + 1] = temp;
				swap = true;
			}
		}
	}

}

This outputs the results in alphabetical order, but I need it in the order of the most votes.

[gunnerfunner]

OK, you've got pt (a). Lets now look at the rest:

(b)

overload copy assignment operator

... this is what you're doing in lines 15-17 of your latest post. It may work because the complier is probably generating a copy assignment operator for you but to be absolutely certain, speacially for user-defined types, you should overload the = operator

(c)

following (b), definitions of copy ctor and dtor

... this refers to the rule of three (or five post C++11 but ignore that for now):
https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)

Finally, your task is to sort by votes but it seems you are sorting by name

[imohamme5]

Could you perhaps give me some code to fix my issue? I think that would make more sense at this point. I understand that I overloaded the copy assignment operator on lines 15-17...However I am still unsure on how to overload the "=" operator.

I have googled this, but still very much confused on how I am to implement this in my code

[Thomas1965]

@gunnerfunner,
why should he need a copy assignment operator, copy ctor and dtor?
He is not using dynamic memory in his candidates class so the ones supplied by compiler are fine..

[cire]

This outputs the results in alphabetical order, but I need it in the order of the most votes.

Then sort based on the number of votes, rather than based on the names. The only issue is that you don't have an accessor for the number of votes like you do for the name of a candidate.

[gunnerfunner]

I overloaded the copy assignment operator on lines 15-17...However I am still unsure on how to overload the "=" operator

They are the same thing (as opposed to move ctor and move assignement operator, you can look these up later if you wish) . As Thomas 1965 points out, in this case, they might not be strictly needed though I have usually user-defined the deep copies and assignments

[JLBorges]

> I overloaded the copy assignment operator on lines 15-17
> how to overload the "=" operator

>> in this case, they might not be strictly needed though.
>> move ctor and move assignement operator, you can look these up later if you wish

Declaring the copy constructor and the copy assignment operator without also declaring the move constructor, the move assignment operator and the destructor is a serious technical error.

By default, a class has 5 operations:
copy assignment
copy constructor
move assignment
move constructor
destructor

If you declare any of those you must consider all and explicitly define or default the ones you want. Think of copying, moving, and destruction as closely related operations, rather than individual operations that you can freely mix and match - you can specify arbitrary combinations, but only a few combinations make sense semantically.

If any move, copy, or destructor is explicitly specified (declared, defined, =default, or =delete) by the user, no move is generated by default. If any move, copy, or destructor is explicitly specified (declared, defined, =default, or =delete) by the user, any undeclared copy operations are generated by default, but this is deprecated, so don't rely on that.

I strongly recommend that if you declare one of these five function, you explicitly declare all.
- Stroustrup http://www.stroustrup.com/C++11FAQ.html#default2

In this case,
>

 class candidates
{
    // ...
 
	int noOfvotes;
	
	string name;
	string votes;
}; 

not adhereing to the rule of zero would be pretty poor style.

Rule of zero
Classes that have custom destructors, copy/move constructors or copy/move assignment operators should deal exclusively with ownership (which follows from the Single Responsibility Principle). Other classes should not have custom destructors, copy/move constructors or copy/move assignment operators.
http://en.cppreference.com/w/cpp/language/rule_of_three

[gunnerfunner]

JLBorges: as Thomas1965 points out, the compiler supplied ones might be sufficient in this case. But, as ever, you're spot on in your observations. Many thanks