snippetcppMinor
Create a struct to store student data and perform statistical analysis on data
Viewed 0 times
studentstructcreateanalysisstoreperformstatisticalanddata
Problem
I had to create a program that had a struct that held data inside of it. I had to read an input file and take the contents and read it into an array of structs. Then, I had to solve for some particular statistical data such as finding the lowest and highest scores from the data. I completed my source code and it runs and gives me the correct output, but I would like second opinions. Please note that the comments are for my own personal benefit. Even though the source code easily allows readers to understand what is going on, I have included them to help myself stay organized.
The instructions for the assignment are down below:
Using functional decomposition, write a program to keep records and perform
statistical analysis for a class of 9 students. There are 3 exams, each worth
100 points, during the semester. Each student is identified by a 4-digit
student ID with a first name and a last name and followed by three exam scores.
You will read the input data from an input file scores.txt (will be posted in
Etudes); and data is in the format (studentID, first name, last name, exam1,
exam2 and exam3).
Each line of data for one student will be read from file and then assigned to a struct variable. Consequently, you will need an array of structs to store all
the data read from the input file. This will be a 1-dimensional array.
Once you read data from the file to your array, you are required to calculate
and display the following statistics for each exam. The statistics to be shown
will be the:
-
lowest score.
-
highest score.
-
average score rounded to 2 decimal place.
-
standard deviation rounded to 2 decimal place.
It will be better to have an array for each statistic of size 3, that is an array for lowest values, and so on.
The ‘xx’ values above needs to be calculated using data provided.
You may assume the scores from the file had been validated and they are all
valid.
Also display
The instructions for the assignment are down below:
Using functional decomposition, write a program to keep records and perform
statistical analysis for a class of 9 students. There are 3 exams, each worth
100 points, during the semester. Each student is identified by a 4-digit
student ID with a first name and a last name and followed by three exam scores.
You will read the input data from an input file scores.txt (will be posted in
Etudes); and data is in the format (studentID, first name, last name, exam1,
exam2 and exam3).
Each line of data for one student will be read from file and then assigned to a struct variable. Consequently, you will need an array of structs to store all
the data read from the input file. This will be a 1-dimensional array.
Once you read data from the file to your array, you are required to calculate
and display the following statistics for each exam. The statistics to be shown
will be the:
-
lowest score.
-
highest score.
-
average score rounded to 2 decimal place.
-
standard deviation rounded to 2 decimal place.
It will be better to have an array for each statistic of size 3, that is an array for lowest values, and so on.
The ‘xx’ values above needs to be calculated using data provided.
You may assume the scores from the file had been validated and they are all
valid.
Also display
Solution
I have found a number of things that could help you improve your code.
Don't abuse
Putting
Avoid the use of global variables
I see that
Use more meaningful variable names
The variable names
Don't use
There are two reasons not to use
General portability
This code could be made portable if, in addition to the changes in the previous point, you omit the Windows-only include files
Prefer arrays to serially numbered variables
The code currently has a great number of variables that are serially numbered such as
Consider efficiency
Instead of making one pass through the data to find the lowest value and then another to find the highest, why not get them both at the same time with a single pass through the data? In fact, all of your statistics can be calculated with a single pass through the data.
Use
Most of the routines in this code take references to data which is not altered. For one obvious example, the
Use better routine names
The function
Return something useful from the subroutines
Every single one of the routines is declared as returning
The advantage is that you can now make a single loop and update both grades and totals simultaneously.
Always pass a size when passing a pointer to an array
As mentioned earlier, when passing the
Don't hardcode file names
Generally, it's not a good idea to hardcode a file name in software, and generally espeically bad if it's an absolute file name (as contrasted with one with a relative path). Instead, it would be better to allow the user of the program to specify the name, as with a command line parameter.
Be more flexible with data
The program currently requires that there are exactly nine student records in the input file. This is not particularly flexible. Instead, it would be nice for the user if one could have any number of students. Strive to provide that flexibility in your software. For example, your program could read in student records until it encounters either the end of the input file or an error in the input data.
Consider adding error checking
Data input usually requires a lot of careful consideration for most software. Checking for malformed input and handling it gracefully often takes a considerable amount of thought, time and code. You might consider isolating the input function to a separate routine and providing error messages that might help the user pin
Don't abuse
using namespace stdPutting
using namespace std at the top of every program is a bad habit that you'd do well to avoid. Know when to use it and when not to (as when writing include headers).Avoid the use of global variables
I see that
SIZE and INFO are declared as global variables rather than as local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. For example, the calcLowest gets the StudentData array as a passed parameter -- the size of that array should also be passed.Use more meaningful variable names
The variable names
SIZE, and INFO are not as descriptive as they could be. Size of what? What info?Don't use
system("PAUSE")There are two reasons not to use
system("cls") or system("PAUSE"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named PAUSE or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example:void pause() {
getchar();
}General portability
This code could be made portable if, in addition to the changes in the previous point, you omit the Windows-only include files
#include "stdafx.h". Prefer arrays to serially numbered variables
The code currently has a great number of variables that are serially numbered such as
std1, std2, etc. This is a very clear indication that you would be better off using an array. The code can then become more compact, more efficient and easier to maintain. For example, consider how much code you would have to change if there were 4 exams instead of 3. Consider efficiency
Instead of making one pass through the data to find the lowest value and then another to find the highest, why not get them both at the same time with a single pass through the data? In fact, all of your statistics can be calculated with a single pass through the data.
Use
const where practicalMost of the routines in this code take references to data which is not altered. For one obvious example, the
print routine takes five different arrays as arguments but alters none of them. You can make this clear by using const in the declaration of the parameters.Use better routine names
The function
getTotal doesn't appear to actually get anything. What it actually does is to calculate the student's total score. A better name might be calculatTotal. Also, it would make sense to have it calculate the total for just one student record. I'd write it like this:int calculateTotal(const StudentData& student)
return student.exam1 + student.exam2 + student.exam3;
}Return something useful from the subroutines
Every single one of the routines is declared as returning
void. Something is wrong there. For example, the calculateTotal routine mentioned above returns the total. You would then use it in a loop outside the routine:for(int i = 0; i < NUM_STUDENTS; ++i) {
students[i].total = calculateTotal(students[i]);
}The advantage is that you can now make a single loop and update both grades and totals simultaneously.
Always pass a size when passing a pointer to an array
As mentioned earlier, when passing the
StudentData array, you should pass the array's size as well. Generally you should get into the habit of doing that for every array that is passed.Don't hardcode file names
Generally, it's not a good idea to hardcode a file name in software, and generally espeically bad if it's an absolute file name (as contrasted with one with a relative path). Instead, it would be better to allow the user of the program to specify the name, as with a command line parameter.
Be more flexible with data
The program currently requires that there are exactly nine student records in the input file. This is not particularly flexible. Instead, it would be nice for the user if one could have any number of students. Strive to provide that flexibility in your software. For example, your program could read in student records until it encounters either the end of the input file or an error in the input data.
Consider adding error checking
Data input usually requires a lot of careful consideration for most software. Checking for malformed input and handling it gracefully often takes a considerable amount of thought, time and code. You might consider isolating the input function to a separate routine and providing error messages that might help the user pin
Code Snippets
void pause() {
getchar();
}int calculateTotal(const StudentData& student)
return student.exam1 + student.exam2 + student.exam3;
}for(int i = 0; i < NUM_STUDENTS; ++i) {
students[i].total = calculateTotal(students[i]);
}Context
StackExchange Code Review Q#88428, answer score: 9
Revisions (0)
No revisions yet.