8
\$\begingroup\$

I'd like this code to be minimized, while displaying the exact same stuff.

Specifically, on how to remove some of the repetitions made within the code (such as "if, else if") etc. using other statements or making the compiler go through it faster, as I feel like I lack the experience and the knowledge to do this without messing up everything (such as using switch & case).

For example, when I test for "tf", I'd like not to be forced to repeat the "average" depending on whether or not we add the "TEZA" grade.


#include <iostream>
using namespace std;
int main()

{
    string sub, tf;
    int m1, m2, m3, m4, sum, TEZA;
    double avg, tzm;

    cout << "SIMPLE AVERAGE CALCULATOR";
    cout << "\n" << "\n" << "Subject at hand?: ";
    cin >> sub;
    cout << "\n" << "Input the FOUR marks you'd like verified: " << "\n";
    cout << "\n" << "M1: ";
    cin >> m1;
    cout << "\n" << "M2: ";
    cin >> m2;
    cout << "\n" << "M3: ";
    cin >> m3;
    cout << "\n" << "M4: ";
    cin >> m4;
    cout << "\n" << "Would you like to include the TEZA grade?(Y/N): ";
    cin >> tf;

    sum = m1 + m2 + m3 + m4;
    avg = (double) sum / 4;

    if (tf == "Y" | tf == "y")
    {
        cout << "What is the TEZA grade?: ";
        cin >> TEZA;
        int tzm = ((double) avg * 3 + TEZA) / 4;
        cout << "\n" << "Your average grade at " << sub << " is " << tzm << "\n"
             << "\n";

        cout << "You got the following mark: ";
        if (tzm >= 9 && tzm <= 10)
            cout << "A" << "\n";
        else if (tzm >= 8 && tzm <= 9)
            cout << "B" << "\n";
        else if (tzm >= 7 && tzm <= 8)
            cout << "C" << "\n";
        else if (tzm >= 6 && tzm <= 7)
            cout << "D" << "\n";
        else if (tzm >= 5 && tzm <= 6)
            cout << "E" << "\n";
        else if (tzm < 5)
            cout << "F" << "\n";

        if (tzm >= 5)
        {
            cout << "DO YOU PASS: " << "\n";
            cout << "Yes." << "\n";
        }
        else
            cout << "No." << "\n";
    }

    else

    {
        cout << "\n" << "Average at " << sub << " is " << avg << "\n" << "\n";

        cout << "You got the following mark: ";
        if (avg >= 9 && avg <= 10)
            cout << "A" << "\n";
        else if (avg >= 8 && avg <= 9)
            cout << "B" << "\n";
        else if (avg >= 7 && avg <= 8)
            cout << "C" << "\n";
        else if (avg >= 6 && avg <= 7)
            cout << "D" << "\n";
        else if (avg >= 5 && avg <= 6)
            cout << "E" << "\n";
        else if (avg < 5)
            cout << "F" << "\n";

        cout << "\n" << "DO YOU PASS?: " << "\n";

        if (avg >= 5)
            cout << "Yes." << "\n";
        else
            cout << "No." << "\n";

    }
}
\$\endgroup\$
1

2 Answers 2

14
\$\begingroup\$

Bug

This calculation looks suspiciously buggy:

int tzm = ((double) avg * 3 + TEZA) / 4;

Why are you taking avg, which is already a double, and casting it to a double? Furthermore, why are you taking the right-hand side, which is a double, and coercing it into an int variable? Note that you had declared double tzm earlier in the code; this int tzm declaration shadows the earlier declaration. Your compiler should have warned you that the tzm on line 8 is unused. (You do compile with warnings enabled, right? It's a good habit.)

In any case, you should write a comment explaining why the tzm formula is the way it is. I don't understand the calculation.

By the way, you can get floating-point division without explicitly casting, simply by writing / 4.0.

Using the bitwise | operator in tf == "Y" | tf == "y" is not really appropriate; it should be a logical || operator.

Optimization

There is absolutely no point in optimizing this code for performance. Nearly all of the time is going to be spent waiting for input; the calculations are trivial.

What you should do, though, is to optimize it for elegance and readability.

The key is to identify functionality within main() that can be extracted out into functions, so that main() no longer has any tedious details. For example, a common pattern is printing a question, and expecting a response from the user. I'd define three functions, to ask the user for string, integer, and yes/no responses.

I would also define a letterGrade() function so that all of the results can be reported using just one statement.

Avoid cluttering your code with variables that don't mean much. I'd get rid of sum and tf, for example. The tzm variable is simply a recalculated average of sorts, so I'd get rid of it too.

Suggested solution

#include <iostream>
#include <string>

std::string ask(const std::string& prompt) {
    std::string answer;
    std::cout << prompt;
    std::cin >> answer;
    return answer;
}

int askInt(const std::string& prompt) {
    int answer;
    std::cout << prompt;
    std::cin >> answer;
    return answer;
}

bool askYN(const std::string& prompt) {
    std::string answer = ask(prompt);
    return answer == "Y" || answer == "y";
}

std::string letterGrade(double grade) {
    return (grade > 10) ? "" :
           (grade >= 9) ? "A" :
           (grade >= 8) ? "B" :
           (grade >= 7) ? "C" :
           (grade >= 6) ? "D" :
           (grade >= 5) ? "E" : "F";
}

int main() {
    std::cout << "SIMPLE AVERAGE CALCULATOR\n\n";
    std::string subject = ask("Subject at hand?: ");

    std::cout << "\nInput the FOUR marks you'd like verified:\n";
    int m1 = askInt("\nM1: "),
        m2 = askInt("\nM2: "),
        m3 = askInt("\nM3: "),
        m4 = askInt("\nM4: ");
    double avg = (m1 + m2 + m3 + m4) / 4.0;

    if (askYN("\nWould you like to include the TEZA grade?(Y/N): ")) {
        int teza = askInt("What is the TEZA grade?: ");
        avg = (3 * avg + teza) / 4.0;
    }

    std::cout << "\nYour average grade at " << subject << " is " << avg << "\n\n"
                 "You got the following mark:" << letterGrade(avg) << "\n"
                 "DO YOU PASS: " << (avg >= 5 ? "Yes." : "No.") << "\n";
}
\$\endgroup\$
10
\$\begingroup\$

Initialize variables

Both sum and avg can be declared and initialized later:

const int sum = m1 + m2 + m3 + m4;
const double avg = sum / 4.0;

Check boundaries in if-else conditions

You have overlapping conditions, e.g.

 tzm >= 9 && <= 10
 …
 tzm >= 8 && <= 9

The latter is effectively only tzm >= 8 && tzm < 9, since tzm == 9 is already handled by tzm >= 9 in your first condition.

Modularize

If I give you an int from 0 to 10, you can print a grade. That's something we can put into its own function:

void print_grade(int mark) {
    if(mark < 0 || mark > 10) {
        std::cout << "Invalid score!\n";
        return;
    }

    std::cout << "You got the following mark: ";

    if(mark >= 9 ){
        std::cout << "A";
    } else if(mark >= 8 ){
        std::cout << "B";
    } else if(mark >= 7 ){
        std::cout << "C";
    } else if(mark >= 6 ){
        std::cout << "D";
    } else if(mark >= 5 ){
        std::cout << "E";
    } else {
        std::cout << "F";
    }
    std::cout << "\nDO YOU PASS: \n";

    if(mark >= 5) {
        std::cout << "Yes.\n";
    } else {
        std::cout << "No.\n";
    }
}

Now we can use

if (tf == "Y" || tf == "y")
{
    std::cout << "What is the TEZA grade?: ";
    std::cin >> TEZA;
    int tzm = (avg * 3.0 + TEZA) / 4.0;
    std::cout << "\n" << "Your average grade at " << sub << " is " << tzm << "\n"
         << "\n";
    print_grade(tzm);
}
else
{
    std::cout << "\n" << "Average at " << sub << " is " << avg << "\n" << "\n";

    print_grade(avg);
}

However, you can also rewrite print_grade as

void print_grade(int mark) {
    static char const * const marks = "FFFFFEDCBAA";
    if(mark < 0 || mark > 10) {
        std::cout << "Invalid score!\n";
        return;
    }

    std::cout << "You got the following mark: " << marks[mark] << "\n";
              << "DO YOU PASS: \n";

    if(mark >= 5) {
        std::cout << "Yes.\n";
    } else {
        std::cout << "No.\n";
    }
}

Or you can use switch (exercise). Either way, in the end you only need to change a single function, not two separate places in your code. That's the lesson you should keep in mind from this small exercise.

Use better names

There is no reason to abbreviate subject to sub. Also, I have no idea what tf stands for.

Use char instead of std::string for single characters

We could write

char get_grade(int mark) {
    static char const * const marks = "FFFFFEDCBAA";
    if(mark < 0 || mark > 10) {
        // Think of an "error" character.
        return '\0';
    }
    returns marks[mark];
}

Same holds for your tf.

Prefer static_cast<type> to C-style (type)casts

Instead of (double) avg use static_cast<double>(avg). Or use floating point literals. By the way, your avg is already a double, so a cast isn't necessary.

Using namespace std

Only use using namespace std; in toy programs, and never in headers.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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