CODEWITHSUNDEEP
javascriptrefactoringconditional-statements
user2906759
user2906759

Reputation: 831

How do you refactor a big chunk of conditionals?

I'm making a game where a player can click an action button. This action button will do completely different things depending on the context.

function doAction() {
    if (standingOnItem) {
        if (itemOnGround === POTION) {
            if (equippiedItem === POTION) {
                // mix potions
                return;
            }

            if (equippiedItem === TORCH) {
                // boil potion
                return;
            }

            // pick up potion
        }

        if (itemOnGround === CHEST && equippedItem === KEY) {
            // open chest
        }

        return;
    }

    if (equippedItem === POTION) {
        // put potion on the ground
    }

    if (equippedItem === TORCH) {
        // put out torch and drop it on the ground
    }

    if (standingOnStaircase && equippedItem === KEY) {
        // move down one level
    }
}

The above is just example code but in my game the doAction function already contains 50 or more conditions. Just knowing in what order to put them has become an issue for every new thing I've added. The problem is that all the different combinations do more or less unique things.

How can you refactor this in a better way? Is there any specific design pattern I can use?

Upvotes: 1

Views: 89

Answers (2)

RobH
RobH

Reputation: 3612

It seems like you could break it down into separate functions based on some top level decision which in this case looks like where the user is standing.

function doAction() {
    if (standingOnGround) {
        doOnGroundAction();
    } else if (standingOnItem) {
        doOnItemAction();
    } else {
        doBasicAction();
    }
}

Then you can have a map of actions based on the other variables:

// Map of item on the ground and item equipped. 
var groundActions = {
    POTION: {
        POTION: function () {},
        TORCH: function () {} 
    },
    CHEST: {
        KEY: function () { 
            // open chest.
        }
    }
};

function doGroundAction() {
    // If there is an action defined in the map execute it, otherwise 
    // perform some default action.
    if (groundActions[itemOnGround] && groundActions[itemOnGround][equippedItem]) {
        groundActions[itemOnGround][equippedItem]();
    } else {
        // Some default action.
    }
}

Upvotes: 1

Jeremy J Starcher
Jeremy J Starcher

Reputation: 23873

There are a few ways to approach this problem.

When you are testing just ONE condition, a switch statement is normal...

switch (action) {
  case "drop":
  // handle drop
  break;
  case "get":
  // Handle get
  break;
};

That said, I really prefer to use JavaScripts objects over switch statements. I think they are just cleaner.

var actions = {
  get: function(obj) {
    // handle get
  },
  drop: function(obj) {
    // handle drop
  }
};

var verb="get";
action[verb](obj);

Upvotes: 1

Related Questions