4
\$\begingroup\$

This is a student module marks calculator that I was asked to write for a uni revision session in year 2.

Is this a good example of modern C++ code? Is there anything I can improve on or need to change to become a better programmer?

#include <iostream>
#include <cstdlib>

float student_results[5][2];

unsigned int input_student_result(unsigned int);

float input_student_weighting(unsigned int);

unsigned int input_exam_result();

float calculate_exam_weighting();

float calculate_module_mark();

int main() {

    // Coursework 1

    for (unsigned int i = 0; i < 4; ++i) {

        student_results[i][0] = input_student_result(i);

        student_results[i][1] = input_student_weighting(i);

    }

    student_results[4][0] = input_exam_result();

    student_results[4][1] = calculate_exam_weighting();

    std::cout << "Module mark = " << calculate_module_mark() << '\n';
    std::cout << '\n';

    system("pause");

    return 0;

}

unsigned int input_student_result(unsigned int n) {

    unsigned int input_validation = 0;

    do {

        std::cout << "Enter coursework " << (n + 1) << " result <0 - 100> = ";
        std::cin >> input_validation;

    } while (!(input_validation >= 1 && input_validation <= 100));

    return input_validation;

}

float input_student_weighting(unsigned int n) {

    float input_validation = 0;

    do {

        std::cout << "Enter coursework " << (n + 1) << " weighting 0.1 or 0.2 = ";
        std::cin >> input_validation;

    } while (!(input_validation == 0.1f || input_validation == 0.2f));

    return input_validation;

}

unsigned int input_exam_result() {

    unsigned int input_validation = 0;

    do {

        std::cout << "Enter exam result <0-100> = ";
        std::cin >> input_validation;

    } while (!(input_validation >= 1 && input_validation <= 100));

    return input_validation;

}

float calculate_exam_weighting() {

    float exam_weighting = (1 - (student_results[0][1] + student_results[1][1] + student_results[2][1] + student_results[3][1]));

    std::cout << "Exam weighting = " << exam_weighting << '\n';

    return exam_weighting;

}

float calculate_module_mark() {

    float module_mark{
            (student_results[0][0] * student_results[0][1]) +
            (student_results[1][0] * student_results[1][1]) +
            (student_results[2][0] * student_results[2][1]) +
            (student_results[3][0] * student_results[3][1]) +
            (student_results[4][0] * student_results[4][1])
    };

    return module_mark;

}
\$\endgroup\$
2
\$\begingroup\$

Here is how you can improve your code:

  • Define a class instead of using an array for unrelated numbers (0 to 100 vs. 0.1 or 0.2)
  • Use a std::vector instead of a fixed size array
  • Prefer double over float, since it has higher precision
  • Prefer integer arithmetic over floating point arithmetic, since it won't lead to strange results that easily
  • Group your global functions into a class, to express that they belong together
  • Add error checking for all calls to std::cin >> var

By the way, you don't look like a beginner since you

  • don't import the complete std namespace
  • use \n instead of std::endl

This is already quite good.

\$\endgroup\$
  • \$\begingroup\$ I am not a C++ programmer, but I would like to know why you say that the higher precision of double is actually going to make a difference here. I don't see anything that needs to have a high precision. \$\endgroup\$ – JakeD Oct 14 '16 at 2:10
  • \$\begingroup\$ Your the only person to actually give me pointers on how to improve from this website everyone else has just said this isn't good in the past, Thanks for giving me constructive feedback! @roland-illig \$\endgroup\$ – Richard Chaos Oct 15 '16 at 17:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.