patterncppModerate
Hour averaging program
Viewed 0 times
averaginghourprogram
Problem
I am looking for tips on improving my short program. I am using
Code:
system("PAUSE") because this was for an assignment. Code:
#include
#include
#include
using namespace std;
void Display(string* , int* , double*, int);
void Percentage(int*, double*, int);
void Intro(string*, int*, int&);
void Highest(int&, string&, double*, string*, int);
int main()
{
int students;
string names[10];
int hours[10];
double percents[10];
int highest;
string most;
Intro(names, hours, students);
Percentage(hours, percents, students);
Display(names, hours, percents, students);
Highest(highest, most, percents, names, students);
system("PAUSE");
}
void Intro(string* names, int* hours, int& students)
{
int team;
cout > team;
cout > names[i] >> hours[i];
}
students = team;
}
void Display(string* names, int* hours, double* percent, int students)
{
cout << setw(20) << "Students";
cout << setw(20) << "Hours Worked";
cout << setw(20) << "% of Total Hours";
cout << endl;
cout << "------------------------------------------------------------" << endl;
for (int i = 0; i < students; i++)
{
cout << setw(20) << names[i];
cout << setw(9) << hours[i];
cout << setw(16) << percent[i];
cout << endl;
}
}
void Percentage(int* hours, double* percent, int students)
{
int total(0);
for (int i = 0; i < students; i++)
{
total += hours[i];
}
for (int i = 0; i < students; i++)
{
percent[i] = double(hours[i]) / total * 100;
}
}
void Highest(int& highest, string& most, double* percent, string* names, int students)
{
highest = percent[0];
for (int i = 0; i < students; i++)
{
if (highest < percent[i])
{
highest = percent[i];
most = names[i];
}
}
cout << most << " worked the most hours." << endl;
}Solution
-
I assume you'll have no more than 10 names/hours/percents, but you could use an
-
If you're allowed to use more of the STL, I'd recommend
For instance, your first loop in
can be done with this function as such (with the array):
If you choose to use
-
I'd slightly tweak the percentage calculation for clarity:
-
Prefer to cast the C++ way:
-
It seems a little weird having
You don't need the first two arguments. They are passed in from
You could then have this:
I assume you'll have no more than 10 names/hours/percents, but you could use an
std::vector in place of these arrays. This will allow you any number of inputs without fear of exceeding the allotted 10. If you don't need this for your assignment, then you can stick with what you have.-
If you're allowed to use more of the STL, I'd recommend
std::accumulate for summing up the values in a container.For instance, your first loop in
Percentage():for (int i = 0; i < students; i++)
{
total += hours[i];
}can be done with this function as such (with the array):
// the 10 corresponds to the array size
// the 0 is the starting value of the accumulator
int total = std::accumulate(hours, hours+10, 0);If you choose to use
std::vector (or other STL container):// functions cbegin() and cend() return const iterators
int total = std::accumulate(hours.cbegin(), hours.cend(), 0);-
I'd slightly tweak the percentage calculation for clarity:
percent[i] = (double(hours[i]) / total) * 100;-
Prefer to cast the C++ way:
// C way
double(hours[i])// C++ way
static_cast(hours[i])-
It seems a little weird having
Highest() display something. You could instead return most, thereby making the function of type std::string. That way, you can display this name from main() or wherever else.You don't need the first two arguments. They are passed in from
main(), but also not modified by any previous functions. I'd remove those two and just use local variables.highest should be of type double since it's being assigned to a percent element. Otherwise, you may receive a "possible loss of data" warning. This is especially important for when the two values are being compared in the loop.You could then have this:
// get the name
std::string most = Highest(percents, names, students);
// display it
std::cout << most << " worked the most hours." << std::endl;std::string Highest(double* percent, string* names, int students)
{
double highest = percent[0];
std::string most;
for (int i = 0; i < students; i++)
{
if (highest < percent[i])
{
highest = percent[i];
most = names[i];
}
}
return most;
}Code Snippets
for (int i = 0; i < students; i++)
{
total += hours[i];
}// the 10 corresponds to the array size
// the 0 is the starting value of the accumulator
int total = std::accumulate(hours, hours+10, 0);// functions cbegin() and cend() return const iterators
int total = std::accumulate(hours.cbegin(), hours.cend(), 0);percent[i] = (double(hours[i]) / total) * 100;// C way
double(hours[i])Context
StackExchange Code Review Q#35941, answer score: 10
Revisions (0)
No revisions yet.