1
\$\begingroup\$

Project description

The program can be tested here

This project is an exercise designed to check if I understand the MVC pattern correctly and apply it correctly.

A quiz is to be designed in which the user should select the correct answer to a question from a given set of answers. If he answers the question correctly, the user receives a point. If he answers incorrectly, the program outputs the points achieved so far and aborts the program execution.

Many tutorials explain the MVC pattern on condition that there is only one class that belongs to the model, and mostly they name it just "Model". So it is interesting for me to know if I have applied the design pattern correctly with several model classes.

I also tried to comment every field and function of a class as it should be except functions that are self-explanatory.

I've also found that to display the data of a class outside of itself requires a lot of data that requires getters and setters that violate the principle of data encapsulation. Is that a weakness of my implementation, or is this a general weakness of the MVC pattern?

Can the controller be equated with something like a main class of a project? Can it include the actual program logic?

The algorithm:

  1. Display a question and its possible answers.
  2. Ask the user for his suggestion.
  3. If he was right: Give the user a point. Go to step 1.
  4. If he was wrong: Exit the program.

Source Files

Main.java

public class Main {
    public static void main(String[] args) {
        Question[] questionList = {
            new Question("In which country Kaiser Wilhelm II was born?",
            new String[] {
                "America",
                "Germany",
                "North Korea",
                "England"
            }, "B"),

            new Question("Which flowers are the most beautiful?",
            new String[] {
                "Tulips",
                "Roses",
                "Lilies",
                "Weeping willows"
            }, "C"),

            new Question("Where does England live?",
            new String[] {
                "On an island",
                "Near poland",
                "In the white house",
                "In he yellow house"
            }, "A"),

            new Question("Who's a free software activist?",
            new String[] {
                "Bill Gates",
                "Donald Trump",
                "Richard Stallman",
                "The GNU operating system"
            }, "C"),


            new Question("Which MMORPG has the most players?",
            new String[] {
                "Arthoria.de",
                "Nostale",
                "GTA 5",
                "World of Warcraft"
            }, "D")
        };

        // problem: the players name can not be initialized with functions from controller 
        // before the controller is initialized
        Player player = new Player("");
        Questions questions = new Questions(questionList);
        View view = new View();

        Controller controller = new Controller(player, questions, view);
        controller.mainLoop();
    }       
}

Player.java

// this class represents a player with a name and a score
public class Player {
    // represents his name
    private String name;
    // represents his score 
    private int score;

    // generates a player with a given name and a score of 0
    public Player(String name) {
        this.name = name;
        score = 0;
    }

    public String getName() {
        return name;
    }

    public int getScore() {
        return score;
    }

    public void setName(String name) {
        this.name = name;
    }

    // increases score of player
    public void scorePoint() {
        score++;
    }
}

Question.java

// this class represents a question with a set of answers
public class Question {
    // represents the question
    private String question;
    // represents four possible answers
    private String[] answers;
    // represents the correct answer with a letter from A to symbol D
    private String correctAnswerLetter;

    // generates a question and needs a string that contains the question, a string list
    // with exactly four answers and a String that has a letter from A to D that points to the 
    // correct answer, throws IllegalArgumentException
    public Question(String question, String[] answers, String correctAnswerLetter) {
        if(answers.length > 4) {
            throw new IllegalArgumentException("answers can only have four strings");
        }
        if(!correctAnswerLetter.equalsIgnoreCase("A") && !correctAnswerLetter.equalsIgnoreCase("B")
        && !correctAnswerLetter.equalsIgnoreCase("C") && !correctAnswerLetter.equalsIgnoreCase("D")) {
           throw new IllegalArgumentException("the letter representing the correct" +
           " string can only have a value from A to D");
        }

        this.question = question;
        this.answers = answers;
        this.correctAnswerLetter = correctAnswerLetter;
    }

    public String getQuestion() {
        return question;
    }

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

    // checks if the given letter equalsIgnoreCase to the correct letter
    public boolean check(String letter) {
        return letter.equalsIgnoreCase(correctAnswerLetter);
    }
}

Questions.java

import java.util.Random;

// this class manages a list of questions
public class Questions {
    // represents a collection of questions
    Question[] questions;

    // generates a set of questions and needs a list of questions
    public Questions(Question[] questions) {
        this.questions = questions;
    }

    // returns a random chosen question from the list
    public Question getRandomQuestion() {
        Random random = new Random();
        int selection = random.nextInt(questions.length);
        return questions[selection];
    }
}

View.java

public class View {
    /* methods for question */

    public void printQuestion(Question question) {
        String[] answers = question.getAnswers();

        System.out.println(question.getQuestion() + "\n");
        System.out.println("A: " + answers[0]);
        System.out.println("B: " + answers[1]);
        System.out.println("C: " + answers[2]);
        System.out.println("D: " + answers[3]);
    }

    /* methods for player */ 

    public void printScoreOfPlayer(Player player) {
        System.out.println(player.getName() + " has reached " + player.getScore() + " points.");
    }

    /* methods for general game logic */

    public void printNameRequest() {
        System.out.print("Your name: ");
    }

    public void printInputRequest() {
        System.out.print("Please chose a letter: ");
    }

    public void printSuccessMessage() {
        System.out.println("That was right!\n");
    }

    public void printGameOverMessage() {
        System.out.println("This was wrong. Game over.");
    }
}

Controller.java

import java.util.Scanner;

public class Controller {
    private Player player;
    private Questions questions;
    private Scanner input;

    private View view;

    public Controller(Player player, Questions questions, View view) {
        this.player = player;
        this.questions = questions;
        input = new Scanner(System.in);

        this.view = view;
    }

    public String getString() {
        return input.next();
    }

    public String getGuessOfPlayer() {
        String guess = input.next();
        if(!guess.equalsIgnoreCase("A") && !guess.equalsIgnoreCase("B") 
        && !guess.equalsIgnoreCase("C") && !guess.equalsIgnoreCase("D")) {
            throw new IllegalArgumentException("Enter A, B, C or D");
        }
        return guess;
    }

    public void mainLoop() {
        view.printNameRequest();
        player.setName(getString());

        while(true) {
            Question actualQuestion = questions.getRandomQuestion();

            view.printQuestion(actualQuestion);
            view.printInputRequest();
            String input = getGuessOfPlayer();
            if(actualQuestion.check(input)) {
                player.scorePoint();
                view.printSuccessMessage();
            } else {
                view.printGameOverMessage();
                view.printScoreOfPlayer(player);
                break;
            }
        }
    }
}
\$\endgroup\$
2
+100
\$\begingroup\$

The MVC is usually misinterpreted (mainly due to Web frameworks). But the goal of the controller is to interpret inputs from the user to update the model while the view display the model. So both the view and controller have a reference to the model.

If you want to have a state-of-the-art implementation, your controller should only call some method to change the view and read user inputs to update the model.

void start() {
    view.askForPlayer();
    String player = readLine();
    model.setPlayer();

    while (!model.isOver()) {
        model.changeQuestion();
        char answer = readAnswer();
        model.answerCurrentQuestion(answer);
    }
}

The view is only used to display things on request of the controller, like you have done. But also when the model changes. This means that the model must expose some fields yes. But is that a violation of the encapuslation principle ? You need to display something, so you need to have acess to it.

void onQuestionChanged() {
    Question question = model.getCurrentQuestion();
    String[] answers = question.getAnswers();
    out.printf("%1$s%n", question.getQuestion());
    for (int i=0; i<answers.length; i++) {
        out.printf("%1$C: %2$s%n", 'A'+i, answers[i]);
    }
    out.print("Please chose a letter: ");
    out.flush();
}

The model contains the state and logic of your application (current question and score). So you can have one model made of different objects (composition). This is not a problem and it is easier to work with one class that encapsulate the access to the underlying objects and fit well with the concept of aggregate in DomainDrivenDesign. I would have a model that use an int to identify the currentQuestion but when the view ask for it, the model return a Question, so it does some encapsulation. You can also read a bit about CQS (without R).

void answerCurrentQuestion(char answer) {
    Question question = questions[currentQuestion];
    if ( question.check(answer) ) {
        score ++;
        onChange(Property.SCORE);
    } else {
        isOver = true;
        onChange(Property.OVER);
    }  
}

Question getCurrentQuestion() {
    return questions[currentQuestion];
}

The business logic must be into your model. But the flow of the program can be in a controller. I have see some cases where there is a super controller that drive the others. Personally I prefer to keep the main method on a separate class and ideally to move the program loop outside of the controllers but in your case, I don't see any good reason to do it.

public static void main(String[] args) {
    QuizzModel model = new QuizzModel(questionList);
    QuizzView view = new QuizzView(model);
    QuizzController ctrl = new QuizzController(model, view);

    ctrl.start(); 
}    

Aside of that, you can improve a bit your code by providing the OutputStream and InputStream to your view and controller, it will be easier to test. I am also a fan of the Formatter when you have many print to the console. And as said by @Flamaker2018 you can improve the validation logic but also be less restrictive on the number of answers.

(Here is a good explanation of the MVC pattern for reference : http://aspiringcraftsman.com/2007/08/25/interactive-application-architecture/)

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

My code review:

View method names have many printXXXX() I assume we could abstract that showQuestion(), or displayName(), promptAnswer()

"A","B","C" appears in many places. I would suggest having

interface UserInput{
   String CHOICE_1 = "A";
   String CHOICE_2 = "B";
   String CHOICE_3 = "C";
   String CHOICE_4 = "D";
}

That would affect different places like:

    System.out.println("A: " + answers[0]);
    System.out.println("B: " + answers[1]);
    System.out.println("C: " + answers[2]);
    System.out.println("D: " + answers[3]);

    new String[] {
     "On an island",
     "Near poland",
     "In the white house",
     "In he yellow house"
    }, "A"),

And validation logic

   if(!correctAnswerLetter.equalsIgnoreCase("A") && !correctAnswerLetter.equalsIgnoreCase("B")
            && !correctAnswerLetter.equalsIgnoreCase("C") && !correctAnswerLetter.equalsIgnoreCase("D")) {
        throw new IllegalArgumentException("the letter representing the correct" +
                " string can only have a value from A to D");
    }

Because if you choose I, II, III, IV later you will need to change many places rather than one.

class Player{
  ......
  public void scorePoint() {
    // scoring player is probably not a place for pojo
    // I would move it to class like quiz or game, but not much logic so far I guess
    score+=POINT_FOR_CORRECT_ANSWER;
  }

  private static final int POINT_FOR_CORRECT_ANSWER = 1;
}

Say there will be difficult and easy questions in one day, would it be an easy change to do? maybe

score += question.getScore() 

Then we will associate question with the score user gets.

I would extract validation logic from question and keep it just as container.

class Question{
  public Question getRandomQuestion() {
    // removed new Random from here, I don't think we need to create 100 objects for 100 questions
    int selection = random.nextInt(questions.length);
    return questions[selection];
  }

  private static final Random random = new Random();
}

For view class, we have many System.out.println() calls - may be it makes sense to abstract that with having System.out.println(String message) as a separate method.

void display(String message);
SystemOutput.display(message -> System.out.println(message));

And calls will be systemOutput.display("your message here").

 public void printQuestion(Question question) {
    String[] answers = question.getAnswers();

    System.out.println(question.getQuestion() + "\n");
    System.out.println("A: " + answers[0]);
    System.out.println("B: " + answers[1]);
    System.out.println("C: " + answers[2]);
    System.out.println("D: " + answers[3]);
}

/* methods for player */

public void printScoreOfPlayer(Player player) {
    System.out.println(player.getName() + " has reached " + player.getScore() + " points.");
}

/* methods for general game logic */

public void printNameRequest() {
    System.out.print("Your name: ");
}

public void printInputRequest() {
    System.out.print("Please chose a letter: ");
}

public void printSuccessMessage() {
    System.out.println("That was right!\n");
}

public void printGameOverMessage() {
    System.out.println("This was wrong. Game over.");
}
\$\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.