Deal with a lot of if-else , switch

Question

Hi! What is the best way to deal with something like this :

if(key==Space)
{
    switch(weapon)
    {
        case GUN:
            p->shotGun();
            break; 
        case BOW:
            p->shotBow();
            break;
    }
}
else if(key==Enter)
{
    //...
}
else if(key==Up)
{
    //...
}

Answer 1

How about levels of nested switch statements? For better readability I would also refactor the second level switches to separate functions.

Answer 2

Split one dimension into two:

switch (key)
{
  case Space:
    ProcessSpaceCmd ();
    break;

  case Enter:
    ProcessEnterCmd ();
    break;

  case Up:
    ProcessUpCmd ();
    break;
}

See what dimension is longer, [key] or [weapon] and take the shortest for the outer switch.

Answer 3

Or if you really want to get rid of if-elses you could use some form of mapping between the key codes and methods. Depending if the key codes are consecutive it could be map or simply an array indexed by the key code like:

KeyMethods[key]();

Answer 4

You might want to think about using the Command and/or Strategy patterns. The Command pattern seems like a good fit for the outer if/else while the Strategy pattern seems like a good fit for the inner switch.

  cmd = Command->GetCommand( key );
  cmd->Perform();

and in Perform for the command associated with the space key

  weapon = PlayState->GetCurrentWeapon();
  weapon->Fire();

Note the latter relies on some global cache/state to hold the current weapon (strategy).

The effect of this would be to move the if/else logic into the factory method where you determine the current command. Choosing which command selects one of the if/else branches. Storing the current weapon in the play state allows you to easily choose which weapon's Fire method to invoke, thus the switch "logic" for which weapon to choose is moved to the weapon selection logic for the state and "disappears" entirely. Each weapon strategy simply knows how to perform it's own "fire" logic.

Answer 5

I'd suggest at least splitting the contents of each If into it's own function.

if(key == Space)
    FureCurrentWeapon();
else if(key == Enter)
    Activate();
else if(key == Up)
    WalkForwards();

An alternative I've used before is to create a base class that can be derived from by each class that receives keyboard input. Keep a list of classes that derive from it, then iterate over that list each time you receive input, passing the key pressed and letting them handle it if they want.

Answer 6

I tend to use a map type expression:

unordered_map<KEY_PRESS,ICommand> myCommands;
unordered_map<KEY_PRESS,ICommand>::const_iterator currentCommand = myCommands.find( key );
if( currendCommand != myCommands.end() ){
    currentCommand->performAction( weapon );
}

Then again, if you made weapons into objects instead of flags you could get away with an overloaded functions pattern.

Answer 7

Since it is C++ you may want to take advantage of the object oriented aspects of it. Make an interface for weapons such as IProjectileWeapon and create a bow and gun class that implements that interface. Then when you call the Shoot() method it will use the correct behavior. That will get rid of the switch statement for the weapon. It would probably be better to have a Character class have a weapon as a member and UseWeapon() as a method. So the UseWeapon() method would call the Shoot() method of the weapon.

For moving, add a move command to the Character class and pass in the direction the character should move.

Answer 8

Another possibility:

class Weapon {
    public:
    abstract void shoot();
}

class Gun: public Weapon {
    public:
    void shoot();
}

class Bow: public Weapon {
    public:
    void shoot();
}

if(key==Space)
{
    weapon.shoot();
}
else if(key==Enter)
{
    //...
}
else if(key==Up)
{
    //...
}

As for the long train of if statements, that's fine. A switch statement would also work IFF you're working with integers. But having the break statement in each one can be distracting, and forgetting it can be disastrous. It's more important if you need speed in an inner-loop for performance-critical code. (else if evaluates each one in turn, until you've found a match. switch jumps directly to the correct option.)

Answer 9

Make your weapon class virtual.