Avoiding the if / switch case constructs to make a code more readable

Question

I recently read a book about clean code, now Im trying to refactor some code that I wrote months ago. There are abstract device class and then some devices which are derived from it, like door, cardreader and so. I have an application that should be interactive via console. The question is if I can somehow refactor code bellow to avoid this long switches, which I use when parsing the command from the console. The user can add, remove, or modify the device, but every device and method has different number of attributes so I dont know if its possible to solve it with polymorphysm.

        string[] command = s.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);


        if(command.Length == 0)
        {
            Console.WriteLine("Neplatny pozadavek");
            return;
        }
        switch (command[0])
        {
            case "add":
                if (command.Length < 3)
                {
                    Console.WriteLine("Neplatny pocet parametru pro operaci add");
                }
                else
                    AddCommand(command, r);
                break;
            case "modify":
                if (command.Length < 3)
                {
                    Console.WriteLine("Neplatny pocet parametru pro operaci modify");
                }
                else
                   ModifyCommand(command, r);
                break;
            case "remove":
                if (command.Length != 3)
                {
                    Console.WriteLine("Neplatny pocet parametru pro operaci remove");
                }
                else
                    RemoveCommand(command, r);
                break;
            case "move":
                if (command.Length != 3)
                {
                    Console.WriteLine("Neplatny pocet parametru pro operaci move");
                }
                else
                    MoveCommand(command, r);
                break;
            default:
                Console.WriteLine("Neznamy prikaz");
                break;
        }
    }

When I pass the operation parsing, I need to parse the device and for each device need to use the if else statement for checking it, the code bellow is just for two device. Is it possible to avoid these switch/if statements somehow to make the code more readable ? Also if I can parse string from console to match a enum better like using switch for all the enum values ? Thank u.

            if (command[2] == "door")
            {
                if (command.Length != 6)
                {
                     return;
                }
                State state;

                switch (command[5])
                {
                    case "locked":
                        state = State.Locked;
                        break;
                    case "open":
                        state = State.Open;
                        break;
                    case "openedforcibly":
                        state = State.OpenedForcibly;
                        break;
                    case "openfortoolong":
                        state = State.OpenForTooLong;
                        break;
                    default:
                        Console.WriteLine("Neplatny stav");
                        return;
                }

                Door d = new Door(deviceId, command[4], state);
                r.AddDeviceToGroup(id, d);
                return;
            }

            if (command[2] == "cardreader")
            {
                  if (command.Length != 6)
                  {
                        return;
                  }
                  CardReader d = new CardReader(deviceId, command[4], command[5], true);
                  r.AddDeviceToGroup(id, d);
                  return;
        }

Answer 1

I get the impression that you have a series of objects that can accept commands. So define them as classes with an interface.

interface ICommandable
{
    void Add(string[] args);
    void Modify(string[] args);
    void Move(string[] args);
    void Remove(string[] args);
}

class Door : ICommandable
{
    public void Add(string[] args) 
    {
        //Implementation
    }

    //etc.
}

class CardReader : ICommandable
{
    //You get the idea
}

You then need a lookup map to map a string to a command or Action:

var commandMap = new Dictionary<string,Action<ICommandable, string[]>>
{
    { "add"    , (c,a) => c.Add(a) },
    { "modify" , (c,a) => c.Modify(a) },
    { "move"   , (c,a) => c.Move(a) },
    { "remove" , (c,a) => c.Remove(a) }
};

You only need to set up the map once, when your program initializes.

You also need a map to store your objects:

var objectMap = new Dictionary<string,ICommandable>
{
    { "door", new Door() },
    { "cardreader", new CardReader() },
};

When it comes time to process a command line, parse the action and the object into separate variables:

var commandText = Console.ReadLine();
var chunks = commandText.Split(' ');
var objectName = chunks[2];
var commandText = chunks[0];
var args = chunks.Skip(3).ToArray();

Now get the object:

var item = objectMap[objectname];

and get the action:

var action = commandMap[commandText];

All that is left is to invoke it:

action(item, args);

That's all there is to it. No switch or if required.

Answer 2

It is absolutely possible to solve it with polymorphism... first declare an interface that sets the command rules like this:

public interface ICommandRule {
    bool IsMatched(string command);
    void ExecuteCommand(string command);
}

Now create command rules for the "add" command, "modify" command, etc by implementing the interface:

public class AddRule : ICommandRule {
    public bool IsMatched(string command) {
        return command == "add";
    }

    public void ExecuteCommand(string command) {
        if (command.Length < 3) {
            Console.WriteLine("Neplatny pocet parametru pro operaci add");
        }
        else
            AddCommand(command, r);
    }
}

Now create a list of all the rules in the calling class. in the executing method apply the Find() method by calling the IsMatched method, once the command matches it will return the needed command, then call the ExecuteCommand() of the matched command. This will now execute what is meant to be inside the case statement. You can also apply this technique to the other case statement.

 public class Class1 {

    List<ICommandRule> _rules;
    public Class1() {
        // put all the rules in a list
        _rules = new List<ICommandRule>() { new AddRule() }; //add other rules
    }

    // the method you are executing your commands
    public void MainMethod() {

        string[] command = s.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
        if (command.Length == 0) {
            Console.WriteLine("Neplatny pozadavek");
            return;
        }
        // instead of the switch
        _rules.Find(rule => rule.IsMatched(command[0])).ExecuteCommand(command[0]);
    }

I think this is one variation of the rules pattern. I've applied it before and it works like magic!

Answer 3

Your switch(command[5]) thisch parses the enum value can be replaced by state = Enum.Parse(typeof(State), command[5], true); (the true flag enables a case-insensitive parsing, as you can read here in the official documentation of the Enum.Parse method).

This was the easy part ;-)

I think you can move the check on command.Length before the switch(command[0]), with a more general error message (or using $"Neplatny pocet parametru pro operaci ${command[0]}" as error message), in order to avoid code duplication between cases.

May be you can turn switch(command[0]) into polymorphism using an approach like that I described here, which I'm used to call set of responsibility. Simplifying the approach my post describes, you can use something like an IDictionary<string, Action<TypeX>> commandHandlers (where TypeX is the type of the r variable in your code) and dispatching the command like follows: commandHandlers[command[0]].Execute(r) (sure, you can test if the dictionary contains command[0] in order to catch errors in the command name provided by the user).

Similarly you can refactor the if(command[2] == "...") chain selecting an Action<...> from another dictionary using command[5] as key; this is an approach similar to polymorphism - refer to my linked post for a pattern which helps you to make the polymorphical approac explicit, which is the solution I prefer.

I hope my contribution can help you in order to analyze the scenario and to find an elegant solution.