2

i am using command line arguments and if conditions are used to check the input values but it is not looking good can i change it to switch but i have no idea how to change it my code is

if (args.Length > 0 && args.Length == 4)
{
    string programName = args[0];
    string file1= args[2];
    string file2= args[3];

    bool flag = false;
    int num= 0;
    bool isNum = Int32.TryParse(args[1].ToString(), out num);

    if (!(programName.Equals("Army")))
    {
        Console.WriteLine("Error"); 
    }

    if (!Int32.TryParse(args[1].ToString(), out isNum ))
    {
        Console.WriteLine("value should be a number");
    }

    if (!File.Exists(file1))
    {
        Console.WriteLine("file 1 does not exist");
    }
    if (!File.Exists(file2))
    {
        Console.WriteLine("file 2 does not exist");
    }
6
  • Did you try researching switch statements? msdn.microsoft.com/en-us/library/06tc147t(v=vs.90).aspx Commented Sep 9, 2013 at 14:51
  • 2
    this doesn't seem a good place to use a switch Commented Sep 9, 2013 at 14:53
  • see if this will help: stackoverflow.com/questions/2282428/… Commented Sep 9, 2013 at 14:53
  • 4
    Why do you use if (args.Length > 0 && args.Length == 4) and not simply if (args.Length == 4) ? Commented Sep 9, 2013 at 14:54
  • 1
    @NirKornfeld: That just stabbed me in the eye too ;p Commented Sep 9, 2013 at 14:55

4 Answers 4

2

A switch statement isn't really called for here. That's useful when you have a single value and need to select from a series of possible mutually-exclusive steps based on that value. But that's not what you're doing here. These aren't a chain of if/else if statements keying off a value, these are more like guard clauses. All of them need to run in order to determine all of the output to show to the user.

You can shorten the code by removing the curly braces:

if (!(programName.Equals("Army")))
    Console.WriteLine("Error"); 
if (!Int32.TryParse(args[1].ToString(), out isNum ))
    Console.WriteLine("value should be a number");
if (!File.Exists(file1))
    Console.WriteLine("file 1 does not exist");
if (!File.Exists(file2))
    Console.WriteLine("file 2 does not exist");

You could also extract these lines of code into their own method, which would make the Main method a little cleaner. You could even extract the conditional checks themselves into very small methods to make it more prose-like for readability. But the conditional structure itself doesn't really need to change.

Sign up to request clarification or add additional context in comments.

Comments

1

You can create class which will be responsible for retrieving and checking your application arguments. E.g. if your application has name Zorg, you can create following class:

public class ZorgConfiguration
{        
    private string num;
    private string programName;
    private string file1;
    private string file2;

    public static ZorgConfiguration InitializeFrom(string[] args)
    {
        if (args.Length < 4)
           throw new ZorgConfigurationException("At least 4 arguments required");

        return new ZorgConfiguration {
            ProgramName = args[0],
            Num = args[1],
            File1 = args[2],
            File2 = args[3]
        };
    }

    // to be continued
}

As you can see, it's responsibility is to hold application settings. It has static method for creating instance of configuration from args array. This method checks if arguments count correct and then initializes each property of configuration class with appropriate argument. Checking argument value moved to properties:

public string ProgramName
{
    get { return programName; }
    private set {
        if (value == "Army")
            throw new ZorgConfigurationException("Error");
        programName = value;
    }
}

public string Num
{
    get { return num; }
    private set {
        int i;
        if (!Int32.TryParse(value, out i))
            throw new ZorgConfigurationException("value should be a number");
        num = value;
    }
}

public string File1 
{
    get { return file1; }
    private set {
        if (!File.Exists(value))
            throw new ZorgConfigurationException("file 1 does not exist");
        file1 = value;
    }         
}

Each property is responsible for verifying corresponding argument value. If value is incorrect, then custom ZorgConfigurationException (that is simply class inherited from Exception) is thrown.

Now main application code looks very clean:

try
{
    var config = ZorgConfiguration.InitializeFrom(args);
    // you can use config.File1 etc
}
catch (ZorgConfigurationException e)
{
    Console.WriteLine(e.Message);
    // exit application
}

Comments

0

I use this class to parse command line arguments, I've found it somewhere, but I can't remember where:

public class Arguments { // Variables private StringDictionary Parameters;

    // Constructor
    public Arguments(string[] Args)
    {
        Parameters = new StringDictionary();
        Regex Spliter = new Regex(@"^-{1,2}|^/|=|:",
            RegexOptions.IgnoreCase | RegexOptions.Compiled);

        Regex Remover = new Regex(@"^['""]?(.*?)['""]?$",
            RegexOptions.IgnoreCase | RegexOptions.Compiled);

        string Parameter = null;
        string[] Parts;

        // Valid parameters forms:
        // {-,/,--}param{ ,=,:}((",')value(",'))
        // Examples: 
        // -param1 value1 --param2 /param3:"Test-:-work" 
        //   /param4=happy -param5 '--=nice=--'
        foreach (string Txt in Args)
        {
            // Look for new parameters (-,/ or --) and a
            // possible enclosed value (=,:)
            Parts = Spliter.Split(Txt, 3);

            switch (Parts.Length)
            {
                // Found a value (for the last parameter 
                // found (space separator))
                case 1:
                    if (Parameter != null)
                    {
                        if (!Parameters.ContainsKey(Parameter))
                        {
                            Parts[0] =
                                Remover.Replace(Parts[0], "$1");

                            Parameters.Add(Parameter, Parts[0]);
                        }
                        Parameter = null;
                    }
                    // else Error: no parameter waiting for a value (skipped)
                    break;

                // Found just a parameter
                case 2:
                    // The last parameter is still waiting. 
                    // With no value, set it to true.
                    if (Parameter != null)
                    {
                        if (!Parameters.ContainsKey(Parameter))
                            Parameters.Add(Parameter, "true");
                    }
                    Parameter = Parts[1];
                    break;

                // Parameter with enclosed value
                case 3:
                    // The last parameter is still waiting. 
                    // With no value, set it to true.
                    if (Parameter != null)
                    {
                        if (!Parameters.ContainsKey(Parameter))
                            Parameters.Add(Parameter, "true");
                    }

                    Parameter = Parts[1];

                    // Remove possible enclosing characters (",')
                    if (!Parameters.ContainsKey(Parameter))
                    {
                        Parts[2] = Remover.Replace(Parts[2], "$1");
                        Parameters.Add(Parameter, Parts[2]);
                    }

                    Parameter = null;
                    break;
            }
        }
        // In case a parameter is still waiting
        if (Parameter != null)
        {
            if (!Parameters.ContainsKey(Parameter))
                Parameters.Add(Parameter, "true");
        }
    }

    // Retrieve a parameter value if it exists 
    // (overriding C# indexer property)
    public string this[string Param]
    {
        get
        {
            return (Parameters[Param]);
        }
    }
}

I use it this way:

var cmdParams = new Arguments(args);

if (cmdParams["File"] != null && parametros["cmdParams"] == "Filename.txt) { }

Hope it helps!

1 Comment

0

Command line arguments can get complicated if there are different functions and arguments..

Best way is to tokenize your arguments, function switch examples are /p /a, or -h, -g etc...Your cmd arg parser looks for these tokens (pattern) - once found you know which cmd it is.. Have switch - case or any other mechanism for this. Also tokenise the other data arguments. Hence you have two sets of arguments - easy to manage.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.