Hi let's start off with the inconsistencies in your code:
#include <iostream>
#include <iomanip>
#include <bits/stdc++.h> //i dont see a need for this
using namespace std; //bad practice. Make sure to not use this in professional projects
int main()
{
const int responseSize = 20;
int vote[responseSize];
const int frequencySize = 6; // 5 people so why 6?
for (int i = 0; i < 21; i++){ //if only 20 people why is there 21 iterations
cout <<"Please enter the next vote: "<< endl;
cin >> vote[i];
}
int frequency [ frequencySize ] = { 0 }; //zero'ing arrays is cool. You didn't zero the vote array.
for ( int answer = 0; answer < responseSize; answer++)
++frequency[vote[answer]]; //dont see a need for preincrementation. default postincrem is fine too (or at least to me personally looks cleaner)
cout << "Rating" << setw(17) << "Frequency" <<endl; //dont see a need for any of the setw() unless you really really need it for something
for ( int rating = 1; rating < frequencySize; rating++) //reading this code makes less sense when you manually change the values you were given. There is a cleaner way. instead 6 people, do 5, start loop from 0 and add 1 to print. more clear.
cout << setw(6) << rating
<< setw(17) << frequency[rating] << endl; //again dont see a point to setw().
return 0;
}
int printmax(int frequency[], int frequencysize)
{
int max =0, winner = 0, i=0; //you create 'i' variable yet you also make one in the loop below.
//also the loop below searched for the highest vote number. Let's say (i know that in this option this wont happen but)
//the max number will be -2, then the answr at the end of the loop will be that max is 0 which would be false. When finding
//a max, min value good way to go round it is to set the value of max to the value of the first item, that way no matter
//what happens or what situation your program will be in, it'll be correct.
for (int i =0; i < frequencysize; i++) //another 'i' variable created
if (frequency[i] > max){
max = frequency[i];
winner = i;
}
cout << "and the winner is: " << i << endl;
return 0;
}
Now Let's see what we can do with it:
#include <iostream>
#include <iomanip>
using namespace std;
int printmax(int frequency[], int frequencysize)
{
int max = frequency[0], winner = 1;
for (int i = 0; i < frequencysize; i++)
if (frequency[i] > max){
max = frequency[i];
winner = i+1;
}
cout << "and the winner is: " << winner << endl;
return 0;
}
int main()
{
const int responseSize = 20;
int vote[responseSize] = { 0 };
const int frequencySize = 5;
for (int i = 0; i < 20; i++){
cout <<"Please enter the next vote: " << endl;
cin >> vote[i];
}
int frequency [ frequencySize ] = { 0 };
for ( int answer = 0; answer < responseSize; answer++)
frequency[vote[answer]-1]++;
cout << "Rating" << "||" << "Frequency" <<endl;
for ( int rating = 0; rating < frequencySize; rating++)
cout << rating+1
<< " " << frequency[rating] << endl;
printmax(frequency, frequencySize);
return 0;
}
Playing around with starting and stopping iterator values may sometimes make your code unfeasible to run. Making sure to keep the loops starting from 0 and ending at your last variable gives more pros than cons, even if you have to add or remove one from the prints or sets in some cases, having it this way enables you or someone else reading your code to clearly see what goes in and what goes out without having to read your code and try to figuire it out themselves. 5 people, 20 votes. So size of 5 and 20 iterations. Also unless you want to forward declare your printmax, define it before your main loop.
P.S. There are many takes for a question such as this, this one was what i'd do if i were given this question. If i screwed something up somewhere let me know in the comments. Hope it helped.
