Quiz Evaluation

I have written a code for following problem:


Write a program to grade a short multiple-choice quiz. The correct
answers for the quiz are:

• C 5. B

• A 6. C

• B 7. C

• D 8. A

Assume that the pass marks are 5 out of 8. The program stores the correct answers in an array. The submitted answers are
specified as program arguments. Let X represent a question that was
not answered on the quiz. Use an enum type to represent the result of
answering a question. The program calculates and prints a report along
the following lines:

And this is my program:

```
public class Grades {

enum Result {
CORRECT, WRONG, UNANSWERED;
}

private static String[] correctAnswers = {"C","A","B","D","B","C","C","A"};
private static int correct,wrong,unanswered;

public static void main (String[] answers) {
String[] args = answers[0].split(",");
if(null == args) {
System.out.println("No questions answered.");
System.exit(-1);
} else if(8 > args.length) {
System.out.println("You must answer all questions. Or write X for the question, you don't want to answer.");
System.exit(-1);
}else if(8 < args.length) {
System.out.println("There are only 8 questions.");
System.exit(-1);
}

showReport(args);
}

private static void showReport(final String[] args) {
System.out.println("Question\tSubmittedAns.\tCorrectAns.\tResult");
for(int i=0;i<args.length;i++) {
System.out.println(i+1+"\t\t"+args[i]+"\t\t"+correctAnswers[i]+"\t\t"+evaluateAnswer(args[i],i));
}

System.out.println("No. of correct answers: "+correct);
System.out.println("No. of wrong answers: "+wrong);
System.out.println("No. of correct answers: "+unanswered);

if(5 < correct) {
System.out.println("The candidate PASSED.");
} else {
System.out.println("

Most of this seems good, so the points are relatively minor.

Conditional order

Your conditional statements are written like null == args or 8 = 5.

Side-effects

Incrementing correct, unanswered and wrong inside evaluateAnswer seems like a pretty clever way of cutting down on the code required to get the final counts. In this case, with a relatively small project and simple problem, it possibly is.

However, in general, even for projects only slightly more involved than this one, things like that are often only clever up until they're suddenly not. This kind of tight-coupling becomes a problem if you need to refactor, or adapt to requirement changes. It also definitely violates the Principle of Least Surprise: evaluateAnswer has a side-effect of changing a static variable, which you would not expect without reading it.

Generally, a method is either expected to return the answer to a question or change some state. Sometimes both, never neither. It's easy to tell whether a method returns the answer to a question because it's not void. But if it's also going to change state, you want to make sure that's very clear from its signature alone. Otherwise it's easy to make assumptions about it which aren't true- for example, that calling it twice will always be equivalent to calling it once.

In this case, while it's a few extra lines of code, I'd suggest adding an extra private static void addResultToCount(Result result) method, and call that inside the showReport for loop. This separates out your concerns nicely and makes it much clearer what's going on where. Then those two methods would look something like:

private static Result evaluateAnswer(String answer, int index) {

        if(answer.equalsIgnoreCase(correctAnswers[index])) {
            return Result.CORRECT;
        } else if(answer.equalsIgnoreCase("X")) {
            return Result.UNANSWERED;
        } else {
            return Result.WRONG;
        }
    }

private static void addResultToCount(Result result) {
        switch(result) {
            case CORRECT:
                correct++;
                break;
            case WRONG:
                wrong++;
                break;
            case UNANSWERED:
                unanswered++;
                break;
        }
    }

Or similar (apologies for any syntax errors with the above, Java isn't my main language)

Expressive names

The names are generally good, but there are places where they could be more expressive. In main, args and answers seem like they should be the other way around. Once those get passed to showReport, they should definitely be interpreted as the more meaningful answers.

Similarly, index in evaluateAnswer doesn't tell us much. I'd suggest questionNumber, but this would be a bit confusing between your 0-indexed array and the 1-indexed actual question numbers. So questionIndex would probably be a good compromise.

Magic numbers

Another minor one for this particular project, but generally a good habit to get into. Instead of hard-coding magic numbers 8 and 5 into your methods, these should be put into constants with expressive names.

In fact, the 8 could instead be taken from the length of correctAnswers, so that you're not expressing the same information in two different places.

StackExchange Code Review Q#70405, answer score: 8