Text menu and validation of the chosen item

Question

I am writing a c# console app that will take a user input from a list of selected options and do things based on the selected option. I have it working right now but just looking over it there seems that I should be able to shorten the code with other methods and make it perform a bit faster and more efficient but for some reason I just can't see it. Am I missing something or is this really the way to be doing this check?

static void Main(string[] arguments)
{
    if (arguments.Length == 0)
    {
        Console.WriteLine("No run mode entered. Usage: JobSightMaintenance <FirstRun | NightlyMaintenance>");
        Console.WriteLine("Please select one of the following options to run:");
        printOptions();

        string userInput;
        bool isValidInput = true;

        do
        {
            userInput = Console.ReadLine();
            int selectedOption;

            try
            {
                selectedOption = int.Parse(userInput);
            }
            catch (Exception ex)
            {
                Console.WriteLine("Invalid Option, please try again");
                printOptions();
                isValidInput = false;
                break;
            }

            switch (selectedOption)
            {
                case 1:
                    isValidInput = true;
                    break;

                case 2:
                    isValidInput = true;
                    break;

                default:
                    Console.WriteLine("Invalid Option, please try again");
                    printOptions();
                    isValidInput = false;
                    break;
            }
        }
        while (!isValidInput);
    }

    if (Debugger.IsAttached) { Console.ReadKey(); }
}

private static void printOptions()
{
    Console.WriteLine("1. Run Intial Data Import");
    Console.WriteLine("2. Nightly Maintenance");
}

Answer 1

Try to keep you Main method simple. It should the core logic of your application. The whole "get option and try again if failed" doesn't really belong in the Main method, and should be extracted into a separated method, not just the print options part.

static void Main(string[] arguments)
{
    if (arguments.Length == 0)
    {
        Console.WriteLine("No run mode entered. Usage: JobSightMaintenance <FirstRun | NightlyMaintenance>");
        Console.WriteLine("Please select one of the following options to run:");

        switch (GetUserOption())
        {
            case 1: 
                // Run Intial Data Import
                break;

            case 2:
                // Nightly Maintenance
                break;

            default:
                // Keep GetUserOption result in sync with this switch
                throw new InvalidOperationException();
        }
    }

    if (Debugger.IsAttached)
    {
        Console.ReadKey();
    }
}

static int GetUserOption()
{
    var options = new Dictionary<int, string>();
    options[1] = "Run Intial Data Import";
    options[2] = "Nightly Maintenance";

    while (true)
    {
        foreach (var option in options)
            Console.WriteLine("{0}. {1}", option.Key, option.Value);

        var input = Console.ReadLine();
        var selectedOption = options.SingleOrDefault(x => x.Key.ToString() == input);

        if (default(KeyValuePair<int, string>).Equals(selectedOption))
        {
            Console.WriteLine("Invalid Option, please try again");
            continue;
        }

        return selectedOption.Key;
    }
}

---

• Do not use int.Parse when you expected it to fail. Use int.TryParse instead, like this :

  while (true)
  {
      var input = Console.ReadLine();
  
      int option;
      if (!int.TryParse(input, out option))
      {
          Console.WriteLine("invalid option");
          continue;
      }
  
      // do something with option
  }
  

---

EDIT: You can also replace the magic numbers with constants to reduce the chance of messing up :

const int InitialDataImportKey = 1;
const int NightlyMaintenanceKey = 2;

static void Main(string[] arguments)
{
    // ...

    switch (GetUserOption())
    {
        case InitialDataImportKey:
            // ...
            break;

        case NightlyMaintenanceKey:
            // ...
            break;
    }

    // ...
}

static int GetUserOption()
{
    var options = new Dictionary<int, string>();
    options[InitialDataImportKey] = "Run Intial Data Import";
    options[NightlyMaintenanceKey] = "Nightly Maintenance";

    // ...
}

Answer 2

Let’s suppose that perfectness is our goal :)

    static void Main(string[] args)
    {
        new Menu("Please select one of the following options to run:")
            .Option("Run Initial Data Import", () => Console.WriteLine("Importing..."))
            .Option("Nightly Maintenance", () => Console.WriteLine("I am the terror (that flaps in the night)!"))
            .Option("Exit")
            .Execute();
    }

Where:

public class Menu 
{
    public Menu(string prompt = "Please choose:")
        : this(prompt, Enumerable.Empty<Option>())
    {
    }

    Menu(string prompt, IEnumerable<Option> options)
    {
        Prompt = prompt;
        Options = options;
    }

    string Prompt { get; }
    IEnumerable<Option> Options { get; }

    public Menu Option(string name, Action action = null) =>
        new Menu(Prompt, Options.Concat(new[] { new Option(name, action ?? (() => {})) }));

    public override string ToString() =>
        Prompt + "\n\r" +
        string.Join("\n\r", Options.Select((o, i) => $"{i+1}. {o.Name}"));

    public void Execute() => Execute(Console.Out, Console.In);

    public void Execute(TextWriter writer, TextReader reader)
    {
        writer.WriteLine(this);
        int i;
        if (int.TryParse(reader.ReadLine(), out i) && --i >= 0 && i < Options.Count())
            Options.ElementAt(i).Execute();
        else
            Execute(writer, reader);
    }
}

And

class Option
{
    public Option(string name, Action action)
    {
        Name = name;
        Execute = action;
    }

    public string Name { get; }
    public Action Execute { get; }
}

Answer 3

I would use

int selectedOption = int.Parse(Console.ReadLine());

Wrap this piece of code into one method

private static void invalidOption()
{
    Console.WriteLine("Invalid Option, please try again");
    printOptions();
    isValidInput = false;
}

Then, I would use a list of options

List<string> optionsList = new List<string>();

I'd add the options to that string

optionsList.add("Run Intial Data Import");
optionsList.add("Nightly Maintenance");

I'd change the printOptions() method this way:

for(int i = 0; i < optionsList.size(); i++)
      Console.WriteLine(i + ". " + optionsList[i]);

Finally, change the switch statement as follows:

if ((selectedOption < optionsList.count) && (selectedOption >= 0))
    isValidInput = true;
else
    invalidOption();

In this way you want to add a new option, you just need to add the string to the list, and you don't have to worry to change the switch statement and to count how many options are available