Cleaning up control flow when dealing wtih command line arguments [C#]
I'm dealing with a program that does plenty of if...else branching based on command line arguments. This is in C# but I'm sure it's applicable to Java, C++ etc. Here's the general outline:
if (args.Length == 0)
{
//do something
}
if (args.Length > 0 && args.Length < 2)
{
Console.WriteLine("Only one argument specified. Need two arguments");
return 0;
}
else if (args.Length > 0 && args.Length >= 2)
{
//Process file - Argument 1
if(args[0].Trim() == PROCESS_OPTION_ONE
|| args[0].Trim() == PROCESS_开发者_运维知识库OPTION_TWO)
{
//Process file - Argument 2
if(args[1].Trim() == PROCESS_CUSTOMER
|| args[1].Trim() == PROCESS_ADMIN
|| args[1].Trim() == PROCESS_MEMBER
|| args[1].Trim() == PROCESS_GUEST
|| args[1].Trim() == PROCESS_USER
)
{
So as you can tell, it's kind of a mess. Is there a design pattern or two that would be most applicable toward cleaning things up some? Command pattern, perhaps? Thanks for the advice and tips.
Stop nesting.
You can switch like (+1) Joel said, or you can just break your logic into clear method calls.
if(args.Length <= 1)
{
Console.WriteLine("Need 2 args kthx");
return;
}
if(args.Length > 2)
{
Console.WriteLine("More than 2 args don't know what do");
return;
}
var arg1 = args[0].Trim();
var arg2 = args[1].Trim();
switch(arg1)
{
case PROCESS_OPTION_ONE:
ProcessOptionOne(arg2);
break;
case PROCESS_OPTION_TWO:
ProcessOptionTwo(arg2);
break;
default:
Console.WriteLine("First arg unknown I give up");
return;
}
then, in your process methods...
private static void ProcessOptionTwo(string argumentTwo)
{
if(argumentTwo == PROCESS_CUSTOMER ||
argumentTwo == PROCESS_ADMIN ||
/* etc blah blah */
}
Keep your methods as simple as possible and break up longer, confusing algorithms into distinct method calls which, through their name, give a clear indication of what they are doing.
I'm partial to using switch statements on the arguments array and setting properties in a configuration class of some kind for each anticipated argument. It appears you're expecting a very specifically formatted argument string rather than allowing set values, you could try:
if(args[0].Trim() == PROCESS_OPTION_ONE || args[0].Trim() == PROCESS_OPTION_TWO)
{
//Process file - Argument 2
switch(args[1].Trim()
{
case PROCESS_CUSTOMER, PROCESS_ADMIN, PROCESS_MEMBER, PROCESS_GUEST, PROCESS_USER:
// Do stuff
break;
default:
// Do other stuff
break;
}
}
My preferred method would be something like
foreach(string arg in args)
{
switch(arg)
{
case PROCESS_CUSTOMER:
// Set property
break;
...
default:
// Exception?
break;
}
}
NOTE: args.Length == 1 is faster than args.Length > 0 && args.Length < 2. It's also a little more readable.
You don't need the else
if you've already returned. That might cut out a lot of your nesting. You could also try using a switch instead of a bunch of nested ifs.
I took the code from this Code Project article a long time ago and made my own version of it to use for command line applications. I did my own modifications to it, like making the class inherit from Dictionary, etc. But the regex part of the code is very good, and makes these sort of command line switches easy as pie.
精彩评论