10
\$\begingroup\$

I made a simple Question "system":

Answer

Stores an answer (string) and a boolean value to check if the answer is correct or not.

public class Answer {

    private String answer;
    private boolean isCorrect;

    public Answer(String answer, boolean isCorrect) {
        this.answer = answer;
        this.isCorrect = isCorrect;
    }

    public String getAnswer() {
        return answer;
    }

    public boolean isCorrect() {
        return isCorrect;
    }

}

Question

I don't know if I should use "Answer[] answers" or "Answers... answers" in the Question constructor.

public class Question {

    private String question;
    private Answer[] answers;

    public Question(String question, Answer... answers) {
        this.question = question;
        this.answers = answers;
    }

    public String getQuestion() {
        return question;
    }

    public Answer[] getAnswers() {
        return answers;
    }

    public boolean isCorrect(String answer) {
        for(Answer tempAnswer : answers)
            if(tempAnswer.getAnswer().equals(answer) && tempAnswer.isCorrect())
                return true;

        return false;
    }

    public boolean isCorrectIgnoreCase(String answer) {
        for(Answer tempAnswer : answers)
            if(tempAnswer.getAnswer().equalsIgnoreCase(answer) && tempAnswer.isCorrect())
                return true;

        return false;
    }
}

SAMPLE

private static Scanner scanner = new Scanner(System.in);

    public static void main(String[] args) {
        Question questionA = new Question("Do chickens fly?",
                                          new Answer("Yes", true),
                                          new Answer("No", false));
        System.out.println(questionA.getQuestion());

        while(true) {
            if(questionA.isCorrectIgnoreCase(scanner.nextLine()))
                break;
            else
                System.out.println("No, that is incorrect. Please try again.");
        }

        System.out.println("Correct.");
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to CodeReview, Vicente. You have quite a good question for a beginner. I hope you get some fine answers. \$\endgroup\$ – Legato May 5 '15 at 0:56
  • 1
    \$\begingroup\$ You can see my similar database version of this at stackoverflow.com/a/5858666/631619 The comments may also help you with some of the same conceptual issues being discussed here \$\endgroup\$ – Michael Durrant May 5 '15 at 10:14
11
\$\begingroup\$

About your primary question: I'd definitely go with Answer... whenever I have the possibility to do so. As you can pass on an array to a varargs parameter, but not varargs to an array parameter, varargs is the better option IMO.

Other comments

Naming

private boolean isCorrect;

It is convention to not prefix boolean fields with is. The name of the method should be is + variable name with upper case starting letter, so:

public boolean isCorrect() {
    return correct;
}

final keyword

It is good practice to mark all possible fields with final, this applies in your case to:

private final String answer;
private final boolean correct; // note the name change

and in the Question class:

private final String question;
private final Answer[] answers;

Mutability

This code might seem harmless to you:

public Question(String question, Answer... answers) {
    this.question = question;
    this.answers = answers;
}

public Answer[] getAnswers() {
    return answers;
}

But the fact is, this is slightly dangerous. Although your Answer class is immutable, this is not. Imagine the following code:

Question questionA = new Question("Do chickens fly?",
     new Answer("Yes", true),
     new Answer("No", false));
questionA.getAnswers()[0] = null;
questionA.getAnswers()[1] = new Answer("Always right", true);

Whoops, now we've cleared one answer and changed the other!

Solution: Make array copies using Arrays.copyOf:

public Question(String question, Answer... answers) {
    this.question = question;
    this.answers = Arrays.copyOf(answers, answers.length);
}

public Answer[] getAnswers() {
    return Arrays.copyOf(answers, answers.length);
}

Possible extension: Make it a score

Your classes supports multiple answers to be correct, that is nicely done. But what if one answer is still "better" than the other? My suggestion is that you use a int or a float for score instead of a boolean correct.

Java 8

If you have Java 8 available, your isCorrect(String answer) method can be improved:

public boolean isCorrect(String answer) {
    return Arrays.stream(answers).anyMatch(a -> a.getAnswer().equals(answer));
}

The same can of course be applied to your isCorrectIgnoreCase method:

public boolean isCorrect(String answer) {
    return Arrays.stream(answers).anyMatch(a -> a.getAnswer().equalsIgnoreCase(answer));
}
\$\endgroup\$
6
\$\begingroup\$

Brackets

I know brackets can be optional, but I would suggest you to use them since the following snippets from your code is hard to read :

     for(Answer tempAnswer : answers)
        if(tempAnswer.getAnswer().equals(answer) && tempAnswer.isCorrect())
            return true;

Instead I would suggest this :

    for (Answer tempAnswer : answers) {
        if (tempAnswer.getAnswer().equals(answer) && tempAnswer.isCorrect()) {
            return true;
        }
    }

Note: This is kind of preference choice. I find it a best practices to always use brackets since it can prevents error and make things more clear, but some people disagree on this.

Answer and question

There is a small problem with your design at the moment, I don't have an obligation to mark an answer as the correct answer. If would create a new Question with only answer that provide the option boolean isCorrect as false, I would loop forever. See this example :

public static void main(String[] args) {
    Question questionA = new Question("Do chickens fly?", new Answer("Yes",
            false), new Answer("No", false));
    System.out.println(questionA.getQuestion());

    while (true) {
        if (questionA.isCorrectIgnoreCase(scanner.nextLine())) {
            break;
        } else {
            System.out.println("No, that is incorrect. Please try again.");
        }
    }

    System.out.println("Correct.");
}

One solution to prevent is to specify the correct answer as a parameter to the Question constructor or check in the constructor that at least one answer as the isCorrect equal to true. If there is no correct answer than just throw an exception.

One other things is, I have no obligation to pass an Answer to the question. You should check for this too.

\$\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.