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

Update 2006/02/22

Uncle Bob said:
"As for the argument about if/else and switch, that's a religious argument IMHO. Switches and if/else chains may be badly abused, and may never be necessary, but sometimes they are pleasant to read. My rule for them is that they should never appear more than once."

My Reply:
"But Uncle Bob, EVERYTIME if we wanna add a new argument type, we have to modify the if-else statement in Args class, and switch-case statement in exception class. We are hit by SAME bullet again and again. I think it violates "Open Close Principle". Yes, i agree the Switch statement are pleasant to read, but if our users ALWAYS want us to support new argument type, we should better avoid if-else/switch statements."

Uncle Bob reply:
"You make a good point. The two switch statements, while not identical, are related; and this violates my rule. On the other hand there is no benefit to turning the switch statement in ArgsException into a table, since it would still need to be modified under the same conditions. I could eliminate the ArgsException switch statement by deferring the messages back to the ArgumentMarshaller, but that couples Messages (UI) with business rules and therefore violates SRP.

I think yours is a good argument for eliminating the messages in the ArgsException class altogether. On the other hand, that just pushes the switch statement out into the user code.

Another solution would be to create a hierarchy of ArgsException derivatives. That would get rid of the switch, but does not completely solve your OCP complaint, since we'd have to add new exception derivatives every time we added a new type.

That leaves us with eliminating the messages and pushing that switch out to the application code. I'm not happy that. So I think the switch in ArgsException, though it violates OCP, helps to keep user code cleaner, and maintains a good separation of concerns (SRP)."