No more Switch-Case/If-Else-If-Else statements

No more Switch-Case/If-Else-If-Else statements

Uncle Bob has written a article about craftsmanship, professionalism, and refactoring (Discussion).

However, after reading this wonderful article (If you have not read it, please start now), I found that Im very uncomfortable with the "Switch-Case" (or if - elseif - elseif - else) statements, just like:

private void parseSchemaElement(String element) throws ArgsException 
{
    char elementId = element.charAt(0);
    String elementTail = element.substring(1);
    validateSchemaElementId(elementId);
    if (elementTail.length() == 0)
        marshalers.put(elementId, new BooleanArgumentMarshaler());
    else if (elementTail.equals("*"))
        marshalers.put(elementId, new StringArgumentMarshaler());
    else if (elementTail.equals("#"))
        marshalers.put(elementId, new IntegerArgumentMarshaler());
    else if (elementTail.equals("##"))
        marshalers.put(elementId, new DoubleArgumentMarshaler());
    else if (elementTail.equals("[*]"))
        marshalers.put(elementId, new StringArrayArgumentMarshaler());
    else
        throw new ArgsException(INVALID_ARGUMENT_FORMAT, elementId, elementTail);
}

And

public String errorMessage() 
{
    switch (errorCode) 
    {
        case OK:
            return "TILT: Should not get here.";
        case UNEXPECTED_ARGUMENT:
            return String.format("Argument -%c unexpected.", errorArgumentId);
        case MISSING_STRING:
            return String.format("Could not find string parameter for -%c.",errorArgumentId);
        case INVALID_INTEGER:
            return String.format("Argument -%c expects an integer but was '%s'.",errorArgumentId, errorParameter);
        case MISSING_INTEGER:
            return String.format("Could not find integer parameter for -%c.",errorArgumentId);
        case INVALID_DOUBLE:
            return String.format("Argument -%c expects a double but was '%s'.",errorArgumentId, errorParameter);
        case MISSING_DOUBLE:
            return String.format("Could not find double parameter for -%c.",errorArgumentId);
        case INVALID_ARGUMENT_NAME:
            return String.format("'%c' is not a valid argument name.",errorArgumentId);
        case INVALID_ARGUMENT_FORMAT:
            return String.format("'%s' is not a valid argument format.",errorParameter);
    }
    return "";
}

Though those code is clean, but everytime if we wanna add a new type of Argument, we have to add new if-else statement in "parseSchemaElement" method, and add new case statement in "errorMessage".  If your Args have to support more than 10 types argument, your if-else(switch-case) statements will be huge and ugly.

Bad smell.

So, can we avoid the Switch-Case statement in our system? Fortunately, the answer is yes: Just use the loop statement instead of it.

Let us see a new version of Args library:
First, the Args Class:

namespace ArgsUtility
{
    public class Args
    {
        //ArgumentName:ArgumentMashaler
        private Dictionary<char, ArgumentMarshalerBase> _myArgumentMashalers = new Dictionary<char, ArgumentMarshalerBase>();
        
        //ArgumentName:ArgumentValue
        private Dictionary<char, string> _myArgumentValues = new Dictionary<char,string>();

        public Args(string schema, string[] argumens) 
        {
            this.ParseSchema(schema);
            this.ParseArgumentStrings(argumens);
        }

        private void ParseSchema(string schema)
        {
            if (string.IsNullOrEmpty(schema))
            {
                throw new Exception("Null Schema");
            }

            foreach (string s in schema.Split(','))
            {
                string schemaNameAndSymbol = s.Trim();
                ValidateSchemaElement(schemaNameAndSymbol);

                char argumentName = schemaNameAndSymbol[0];
                char argumentTypeSymbol = schemaNameAndSymbol[1];

                ArgumentMarshalerBase amb = SupportedArguments.Instance.GetArgumentMarshaler(argumentTypeSymbol);
                this._myArgumentMashalers.Add(argumentName, amb);
            }
        }

        private  void ValidateSchemaElement(string s)
        {
            //each schema element should like this: 'x*', only two char
            if (s.Length != 2)
            {
                throw new Exception("Bad Schema: " + s);
            }
        }

        private void ParseArgumentStrings(string[] values)
        {
            IEnumerator ie = values.GetEnumerator();

            //ugly code, i think, but all tests can pass, so let me ignore it at this time.
            while (ie.MoveNext())
            {
                if (ie.Current.ToString().StartsWith("-"))
                {
                    char argumentName = ie.Current.ToString()[1];
                    
                    if (ie.MoveNext())
                    {
                        string argumentValue = ie.Current.ToString();
                        this._myArgumentValues.Add(argumentName, argumentValue);
                    }
                }
            }
        }

        public object this[char argumentName]
        {
            get
            {
                ArgumentMarshalerBase amb;
                if (this._myArgumentMashalers.ContainsKey(argumentName))
                {
                    amb = this._myArgumentMashalers[argumentName];
                }
                else
                {
                    throw new Exception(string.Format("Argument {0} unexpected.", argumentName));
                }

                string argumentValue;
                if (this._myArgumentValues.ContainsKey(argumentName))
                {
                    argumentValue = this._myArgumentValues[argumentName];
                }
                else
                {
                    throw new Exception(string.Format("Can't find {0} parameter for {1}.", amb.ArgumentType, argumentName));
                }

                return amb.ParseInputValue(argumentValue);
                
            }
        }
    }
}

Look, in new version of "ParseSchema" method, the if-else statements were gone, and a static method which named "SupportedArguments.Instance.GetArgumentMarshaler(argumentTypeSymbol);" instead of them.

So, let see how "GetArgumentMarshaler(string argumentTypeSymbol)" works:

namespace ArgsUtility.ArgumentMarshaler
{
    class SupportedArguments
    {
        private List<ArgumentMarshalerBase> _commonArgs = new List<ArgumentMarshalerBase>();
        readonly public static SupportedArguments Instance = new SupportedArguments();

        private SupportedArguments()
        {
            this._commonArgs.Add(new BooleanArgumentMarshaler());
            this._commonArgs.Add(new IntegerArgumentMarshaler());
            this._commonArgs.Add(new DoubleArgumentMashaler());
            this._commonArgs.Add(new StringArgumentMashaler());
            //todo: add more argumentMarshalers
        }

        public ArgumentMarshalerBase GetArgumentMarshaler(char typeSymbol)
        {
            foreach (ArgumentMarshalerBase amb in this._commonArgs)
            {
                if (amb.IsSymbolMatched(typeSymbol))
                {
                    return amb;
                }             
            }
            throw new Exception(string.Format("Unknow Argument Symbol:{0}.",typeSymbol));
        }

    }
} 

"GetArgumentMashaler" method check all supported argumentMashalers in _commonArgs collection, check them one by one and find the correctly Argument Mashaler.

And the ArgumentMarshalerBase is:

namespace ArgsUtility.ArgumentMarshaler
{
    abstract class ArgumentMarshalerBase
    {
        private char _schemaSymbol;
        public ArgumentMarshalerBase(char schemaSymbol)
        {
            this._schemaSymbol = schemaSymbol;
        }

        public bool IsSymbolMatched(char targetSymbol)
        {
            return targetSymbol == this._schemaSymbol;
        }

        public object ParseInputValue(string value)
        {
            try
            {
                return this.DoParse(value);
            }
            catch (FormatException ex)
            {
                //catch all format problems
                throw new Exception(string.Format("'{0}' is an invalid {1} format.", value, this.ArgumentType),ex);
            }

        }

        abstract protected object DoParse(string value);
        abstract public string ArgumentType
        {
            get;
        }
                      
    }
}

The BooleanArgumentMarshaler implement as below:

namespace ArgsUtility.ArgumentMarshaler
{
    class BooleanArgumentMarshaler:ArgumentMarshalerBase 
    {
        public BooleanArgumentMarshaler()
            : base('?')
        {
        }

        protected override object DoParse(string value) 
        {
            return Boolean.Parse(value);
        }

        public override string ArgumentType
        {
            get
            {
                return "Boolean";
            }
        }
    }
}

Pretty simply，huh? Now, if we wanna add new type of argument, we just need:
1. Write a new class which implement abstract class: ArgumentMashalerBase
2. In the new class constructor, set the a symbol for it. This symbol will be used in schema string. E.g. the symbol of boolean is "?" and symbol "#" is for integer type argument.

You can will see the new argument will works well.

[Test]
        public void TestComplexArguments()
        {
            /**//* this schema includes 7 arguments:
             * Integer Arguments: i, j
             * Boolean Arguments: b
             * Double Arguments: d, x
             * String Arguments: m, n
             */
            string schema = "i#,b?,j#,d&,m*,n*,x&";

            //the argument values
            string[] values = new string[] {
                "-i", "32",
                "-d", "33.4",
                "-m", "string 1", 
                "-b", "false",
                "-j", "53",
                "-x", "11.2",
                "-n", "string 2"};

            Args args = new Args(schema, values);

            Assert.AreEqual(32, (int)args['i']);
            Assert.AreEqual(33.4, (double)args['d']);
            Assert.AreEqual("string 2", (string)args['n']);
            Assert.AreEqual("string 1", (string)args['m']);
            Assert.AreEqual(53, (int)args['j']);
            Assert.IsFalse((bool)args['b']);
            Assert.AreEqual(11.2, (double)args['x']);
        }

Is that all? NO! Please, do not forget add new test cases for your new argumet ;)

Full source code and unit test are HERE. Any suggest is welcome.

linkcd
2006/02/21

posted on 2006-02-21 10:50 linkcd 阅读(855) 评论(3)  编辑 收藏 收藏至365Key

Feedback

# re: No more Switch-Case/If-Else-If-Else statements 2006-02-21 16:46 zdnet

我觉得反倒降低了代码的可读性,加大了维护难度  回复
  

# re: No more Switch-Case/If-Else-If-Else statements 2006-02-21 22:05 老翅寒暑

我觉得没有必要太过于偏执，switch也有它合适的用途的。  回复
  

# re: No more Switch-Case/If-Else-If-Else statements 2006-02-22 09:35 不会飞的鱼

同意上面两位的评论，我觉得楼主的代码反而把原来的本意搞复杂了。  回复