2

I have the following code:

public void actionPerformed(ActionEvent e) {
    String userInput = commandInput.getText();
    if (currentLevel == 0) {
        if (userInput.equals(answers.getIntroAnswers().get(0)) || userInput.equals(answers.getIntroAnswers().get(1))) {
            messageDisplay.append("\n \n" + userInput + "\n");
            commandInput.setText("");
            messageDisplay.append("\n" + messages.getNextMessage());
            currentLevel++;
            getCurrentLevel();
        } else {
            messageDisplay.append(notValid());
        }
    } else if (currentLevel == 1) {
        // do the same as above but with the next set of answers
    }
}

What I'd like to do is somehow separate this action into it's own class and call the method /constructor within that class to do this checking else I will be stuck using nested if's and it will become very messy and hard to understand. Would I be right in thinking a method to take parameters of currentLevel and userInput in order to test the userInput against the corresponding answers based on the currentLevel? Below is a link to the rest of the classes involved:

https://github.com/addrum/TextGame.git

Adam Short
  • 498
  • 7
  • 28

3 Answers3

2

Would I be right in thinking a method to take parameters of currentLevel and userInput in order to test the userInput against the corresponding answers based on the currentLevel?

No. In fact, you probably want to avoid passing the current level as an explicit parameter. If you've got the level as a parameter, you will probably end up just pushing the "multiple nested ifs" into another class.

I think you need to write it like this:

    InputChecker[] levelChecker = ... create an array of checker instances
    ....
    levelChecker[currentLevel].check(userInput);

Then you need to create a class (possibly anonymous) to implement the checking for each level. Note that if you needed to you could supply the level number to a checker class via a constructor parameter and have it save it in a private instance variable.

You could expand/generalize the InputChecker interface to include other level-specific behaviour. Or indeed make this part of a Level interface.


"Is this taking the currentLevel and comparing the userInput to the current level?"

No. In my example code above it is calling a method on the InputChecker instance to do the checking. Since there are different InputChecker instances for each level, they can check different answers ... or whatever.

But if the only difference between the "input check" behaviours for each level is that they check against a different set of answers then:

levelAnswers = answers.getAnswersForLevel(currentLevel);
for (String answer : levelAnswers) {
    if (userInput.equals(answer)) {
       // blah blah blah
    }
}
Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • I do kind of want the if's in another class as the class it's in is for the Applet and I'm trying to keep that clean and just for set up. >levelChecker[currentLevel].check(userInput); Is this taking the currentLevel and comparing the userInput to the current level? Because the userInput needs to be checked against an array of answers per message so I don't think this would work which is why I thought about passing in the currentLevel as this could then be referred to the correct message which in turn can then check the userInput against the answers matching that message. – Adam Short Jun 09 '13 at 00:02
  • I get each individual part of your answers but I can't get my head around how to actually use each bit. I think I'm overcomplicating it but could you possibly branch the Git repo and do it that way with comments? – Adam Short Jun 09 '13 at 11:41
0

Why not create the method in the same class rather than having a different class to do that, considering other variables that method uses such as,

        messageDisplay.append("\n \n" + userInput + "\n");
        commandInput.setText("");
        messageDisplay.append("\n" + messages.getNextMessage());
        currentLevel++;

So I'd suggest creating the method in same then call it from actionPerformed

       public void checks()
       {
         String userInput = commandInput.getText();
          if (currentLevel == 0) {
            if (userInput.equals(answers.getIntroAnswers().get(0)) ||    userInput.equals(answers.getIntroAnswers().get(1))) {
              messageDisplay.append("\n \n" + userInput + "\n");
              commandInput.setText("");
              messageDisplay.append("\n" + messages.getNextMessage());
              currentLevel++;
              getCurrentLevel();
            } else {
                messageDisplay.append(notValid());
               }
           } else if (currentLevel == 1) {
                // do the same as above but with the next set of answers
               }
        }

Then call it from actionPerformed

     public void actionPerformed(ActionEvent e)
     {
       check():
     }

So now you if's are handle in a seperate method.

Sello
  • 297
  • 1
  • 8
0

To my eyes, since you are talking about levels so much, you probably should have a class that represents a level. Actually, since you obviously have more than one level, which acts slightly differently, you have two approaches.

  1. Have a Level interface, and then make a class per level.

or

  1. Have a Level class, with a constructor that hides the level number within the class.

After that, you can switch polymorphicly instead of nested if statements (or if's cousin, the switch statement).

Level level = currentLevel;
while (level != null) {
  level.doStuff;
  level = level.getNextLevel();
}
Edwin Buck
  • 69,361
  • 7
  • 100
  • 138