4
\$\begingroup\$

I'm working on cleaning this code up a bit and was hoping for some suggestions. Right now it works well enough on a basic level, but it doesn't check for out of bounds numbers or improper input.

Also, one of my big concerns is too much nesting. Does the code look ugly? Any suggestions on how to clean it up?

#include <iostream>
#include <fstream>
#include <cmath>
using namespace std;

int numClasses();

int main()
{
    double finalResult[4], daysUntilDue, result, numOfProblems[199], numOfDays[99];
    string assignmentName[4];
    int numOfClasses, numItems[3], i, j;
    string classNames[3];

    numOfClasses = numClasses();

    ofstream myFile;
    myFile.open ("/Users/nicolasparsons/Desktop/HWAverages.txt");

    for (i = 0; i < numOfClasses; i++)
    {
        cout << "What is the name of class #" << i + 1 << "? (do not use spaces): ";
        cin >> classNames[i];
        cout << "How many assignments do you have for " << classNames[i] << "? (must be 5 or less): ";
        cin >> numItems[i];
        for (j = 0; j < numItems[i]; j++)
        {
            cout << "Enter a name for assignment #" << j + 1 << "? (do not use spaces): ";
            cin >> assignmentName[j];
            cout << "Enter number of problems for " << assignmentName[j] << " (must be 200 or less): ";
            cin >> numOfProblems[j];
            cout << "Number of days until this assignment is due (must be 100 or less): ";
            cin >> numOfDays[j];
            finalResult[i] = numOfProblems[j] / numOfDays[j];
            //cout << finalResult[i] << endl;
            myFile << classNames[i] << endl << assignmentName[j] << ": " << finalResult[i] << " per day until due date. " << endl << endl;
        }
    }

return 0;
}

int numClasses()
{
    int result;
    cout << "How many classes do you have? (must be 4 or less): ";
    cin >> result;
    return result;
}
\$\endgroup\$
2
\$\begingroup\$

Here are some things that may 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.

Make sure you have all required #includes

The code uses std::string but doesn't #include <string>. It's important to make sure that your code includes all required library headers both the reliably compile and to give the reader a clue as to what is within the program.

Eliminate unused variables

The variables daysUntilDue and result in your code are defined but never used. Since unused variables are a sign of poor code quality, you should seek to eliminate them. Your compiler is probably smart enough to warn you about such things if you know how to ask it to do so.

Use objects

Since you're writing in C++, what would make more sense to me would be to create a grades object. Then each operation would be very naturally expressed as an operator on the object.

Decompose your program into functions

Almost all of the logic here is in main in one rather long and dense chunk of code. It would be better to decompose this into separate functions.

Eliminate magic numbers

The constants 3, 4, 99 and 199 are used in multiple places. It would be better to have them as named const values so that it would be clear what those numbers represent.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

\$\endgroup\$

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.