HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Create a struct to store student data and perform statistical analysis on data

Submitted by: @import:stackexchange-codereview··
0
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

Solution

I have found a number of things that could help you improve your code.

Don't abuse using namespace std

Putting 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 practical

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