Click here to Skip to main content
15,884,298 members
Please Sign up or sign in to vote.
1.00/5 (2 votes)
See more:
I would like to turn my code into a simpler version. The current seems too complex, and/or too slow to create hundreds of commands. Please explain other ways. I already attempted to put it into a switch statement, but it doesn't work because it's a method, not variable. Example of what I tried:

C#
switch (rawCmd.Contains("Help")) {

}

(And obviously I would have put the cases in there.)

My current code is down there v

Thanks a lot in advance!!

What I have tried:

C#
public static String ParseCmd(String rawCmd)
        {
        if (rawCmd.Contains("help") || rawCmd.Contains("Help") || rawCmd.Contains("HELP"))

        {
            return "Type a command!";
        }
        return null;
    }
Posted
Updated 19-May-16 7:55am
Comments
Peter_in_2780 18-May-16 22:33pm    
Two suggestions:
1. If you would accept HElp and other such variants of case, then convert your rawcmd to one case.
2. Take a look at the Dictionary class to hold your commands and return strings
VenHayz 19-May-16 10:23am    
Thank you very much, I'll look into it before I select any answers.

First of all, your switch statement doesn't work because String.Contains() returns a bool.
C#
switch (rawCmd.Contains("Help"))
{
 
}

should be
C#
switch (rawCmd)
{
    case : "help" : ..; break;
    case : "heLP" : ..; break;
    case : "HeLP" : ..; break;
    etc. 
    default: // maybe throw an exception
}

But as you see you will have a lot of cases, and this just for one command.
So better to do like suggested in the comment and use either .ToLower or .ToUpper.
C#
if (rawCmd.ToUpper().Equals("HELP"))
{
    // Do something
}

or if you have many commands
C#
switch (rawCmd.ToUpper())
{
    case "HELP" : ...; break;
    etc. 
    default: // maybe throw an exception
}
 
Share this answer
 
v2
Comments
VenHayz 19-May-16 10:25am    
This is definitely the most helpful answer, so I will vote this one to answer my problem. Thanks a lot. I'm getting a lot more love here, than on StackOverflow...
George Jonsson 19-May-16 19:14pm    
You are welcome.
Since you'll have "hundreds, maybe thousands of commands" then I'd suggest using a Dictionary<string, string>. Something like:
C#
private static Dictionary<string, string> CommandMap = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase) {
    { "Help", "Type a command!" },
    { "Quit", "Goodbye!" },
    { "MyCommand", "My Command response" },
    // etc... { commandstring, responsestring }
  };
public static string ParseCmd(string rawCmd)
{
  string response;
  if (!CommandMap.TryGetValue(rawCmd, out response)
  {
    // here's where you could respond to an unrecognized command
    // or you could just let ParseCmd return null and handle it somewhere else.
  }
  return response;
}


But you implied that you had subcommands so you might need to have a different structure, where each top-level command returns the information necessary to parse the specific subcommand. What that information is depends on what you're doing.
I suspect some sort of tree structure.
 
Share this answer
 
Comments
VenHayz 19-May-16 16:16pm    
This another great answer, and I thank you very much. I don't think I can say that I'll change my decision, so I'll tell you that I'll use each version and tell you what I think.
Matt T Heffron 19-May-16 16:19pm    
With lots of commands, the if or switch solutions take (on average) time proportional to the number of commands to "consider". O(n)
The solution with Dictionary is constant. O(1)
If you start to check for cases, you need to check all combinations.
help, helP, heLp, heLP, hElp, hElP, hELp, hELP, Help, HelP, HeLp, HeLP, HElp, HElP, HELp and HELP.
The best solution is to force to lower case or upper case, your choice.
C#
public static String ParseCmd(String rawCmd)
        {
        string TmpCmd= rawCmd.ToUpper();
        if (TmpCmd.Equals("HELP")
        {
            return "help!";
        }
        else if (TmpCmd.Equals("QUIT")
        {
            return "Quit!";
        }
        else if (TmpCmd.Equals("MYCOMMAND")
        {
            return "My Command!";
        }
        else
        {
            return "Unknown Command!";
        }
        return null;
    }


[Update]
if and switch perform the same. The difference is that if allow more sophisticated conditions.
For hundred or thousands of commands, other techniques are to be used to prevent having to check every possibilities.
More details would be needed to determinate which technique is best.
 
Share this answer
 
v3
Comments
Matt T Heffron 19-May-16 2:24am    
Didn't you mean .ToUpper()?
Or all the strings you compare to should be all lowercase...
Patrice T 19-May-16 2:34am    
Ooops :)
VenHayz 19-May-16 10:20am    
This is very helpful, but I think that going with "if..else" is a lot more junky than doing a switch statement. This is a very good answer, but I'm looking to hold hundreds, maybe thousands of commands (under sub-commands). Thanks a lot though!
Matt T Heffron 19-May-16 13:56pm    
See my Solution 4 for thoughts on many, many commands efficiently.
You can do like below...
C#
if(rawCmd.ToLower().Equals("help")){
    // Codes here
}
 
Share this answer
 
v2
Comments
VenHayz 19-May-16 10:24am    
Very simple, I like it.
Thanks. Then accept the solution. :)

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900