Replacing if else if statements with something more efficient

Question

I'm trying to learn coding by building a simple text game. The end game is going to have 4 rooms. You'll start in room 1, exit west to room 2, exit south to room 3, and finally exit east in room 4. (clockwise).

Anyway, my starting code is from a YouTube tutorial I found that consists of all if / else if statements. I already see that's terribly inefficient. My question is how do I improve this code?

I'm guessing I should make each room and it's contents an object (ie. Room 1 has a sword in it, so the object would contain the location of the room and the sword). I'm also guessing if I have a monster in a room, he'd be his own object.

My problem is if the above is correct (object) - I don't understand how to use the object once I create it. ie. if the user types "take sword" how do I call the object to do that?

If I'm on the complete wrong track, please point me back in the right direction.

Here's the current code for the first room:

$("form").submit(function() {
    var input = $("#commandLine").val();

    function check() {
        check = true;
    }

    if (input == "help") {
        $("#messageHelp").clone().insertBefore("#placeholder").fadeIn(1000);
        check();
    }

    if (input == "take sword" && currentRoom == "nCorridor") {
        $("<p>You picked up a sword.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
        check();
    }
    else if (input == "take sword" && currentRoom != "nCorridor") {
        $("<p>The sword is not here.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
        check();
    }
    else if (input != "take sword" && input != "help") {
        $("<p>I don't understand " + input +  ".</p>").hide().insertBefore("#placeholder").fadeIn(1000);
    }

    $("#commandLine").val("");
});

Ideally, I'd like to eliminate or greatly reduce my need to use if and else if statements for something more efficient.

There's nothing inherently wrong with your multiple `if` statements. My only suggestions would be to DRY up the repeated logic (`.hide().insertBefore("#placeholder").fadeIn(1000);`) and remove the seemingly useless `check()` function — Rory McCrossan, Oct 28 '15 at 17:11
Maybe look into `switch`, but it's not more efficient as it only works for one evaluation at a time, not multiple evaluations like with your 'room', although you could always put that another function with another switch statement? — somethinghere, Oct 28 '15 at 17:13
@RoryMcCrossan - I agree about the repeated logic. I was wondering if I could take all that and put it in a function then just call that function? As for the check() - It's needed to make the "I don't understand" statement work. See here: http://stackoverflow.com/questions/33379776/need-help-correcting-a-function-in-text-game — megler, Oct 28 '15 at 17:18
There are a lot more issues here - for one, what about capitalisation? What if I type a space after my typing? You will have to consider this at a lower level, breaking apart the sentence and evaluating words and how they go together. This is harder than it seems :) — somethinghere, Oct 28 '15 at 17:22
what others have said.. + perhaps have separate functions/methods for each action type? — Brad Kent, Oct 28 '15 at 17:24
Code could be reduce to a couple of line since most of it is just copy paste. — epascarello, Oct 28 '15 at 17:25
Am I the only one who realizes that West -> South -> East is counter-clockwise? — Stan Shaw, Oct 28 '15 at 17:42
Sorry, @Stan. I had a directional blonde moment. You're correct. — megler, Oct 28 '15 at 17:47

Guffa · Accepted Answer · 2015-10-28T17:34:13.950

First let's improve the logic in the if statements to reduce the duplicate conditions, to see how far that gets you:

if (input == "help") {

  $("#messageHelp").clone().insertBefore("#placeholder").fadeIn(1000);
  check();

} else if (input == "take sword") {

  if (currentRoom == "nCorridor") {
    $("<p>You picked up a sword.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
    check();
  } else {
    $("<p>The sword is not here.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
    check();
  }

} else {

  $("<p>I don't understand " + input +  ".</p>").hide().insertBefore("#placeholder").fadeIn(1000);

}

Another way to determine the action depending on input is using a switch, which may be more useful when you get more options:

switch (input) {

  case "help":
    $("#messageHelp").clone().insertBefore("#placeholder").fadeIn(1000);
    check();
    break;

  case "take sword":
    if (currentRoom == "nCorridor") {
      $("<p>You picked up a sword.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
      check();
    } else {
      $("<p>The sword is not here.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
      check();
    }
    break;

  default:
    $("<p>I don't understand " + input +  ".</p>").hide().insertBefore("#placeholder").fadeIn(1000);

}

To go on and using objects for keeping track of items, you could create an object for the sword (with just the location for now):

var sword = {
  room: "nCorridor"
};

In the code you could use the object like this:

  if (currentRoom == sword.room) {
    $("<p>You picked up a sword.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
    check();
  } else {
    $("<p>The sword is not here.</p>").hide().insertBefore("#placeholder").fadeIn(1000);
    check();
  }

From there you can add more properties and methods to items. The objects might for example have methods that you can use to determine what you can do with them, like that the item apple can be eaten, but the item sword can not.

Thanks @guffa! I see what you did with that first block. It makes more sense. I also pulled out the "I don't understand" portion and made it a function as that will be used throughout the game, not just in 1 room. I'll need to learn more about switches to see if that will work for me. Right now, if statements are fine because the game is small, but if this were to be a larger or more complicated game then it would quickly get out of hand. That's why I'm trying to learn a better way now. Do you have any idea why I can't incorporate the check() into the longer function line? (.hide()... — megler, Oct 28 '15 at 17:42
To add to the above - chrome console says check() not a function. I thought before the function and variable have the same name, but I changed the function name to something else with the same result. :-/ — megler, Oct 28 '15 at 17:45
What is the `check` function supposed to do? Right now it is just replacing itself with the value `true`, so the function will only be possible to call once. If you have declared a variable `check` also then that would be shadowed by the function, but if you have separate names for them that would work fine. I'm not sure how you would want to incorporate the function call in the jQuery calls, but if you for example want to use it as a callback in the `fadeIn`, that would work just fine: `.fadeIn(1000, check);`. — Guffa, Oct 28 '15 at 19:08
A problem in the original code was if you typed in "help" the help statement would run, but it would also say "I don't understand help." I was following a YouTube video that solved the problem by creating the check() function and using it to make sure that each statement (take sword, help, etc) ran correctly without the "I don't understand" also being inserted. I'm sure there's a better way, I'm learning this as I go! — megler, Oct 29 '15 at 00:07
@megler: Look at how the `if ... else if ... else` and the `switch` above handles that, they don't have that problem. — Guffa, Oct 29 '15 at 03:06

Replacing if else if statements with something more efficient

1 Answers1