Optimizing simple menu in console app

I want to optimize code in my simple app that I'm writing to learn C#. I feel it's really badly optimized in terms of good coding practises. Could anyone take a look at it and give me a hint to make it look better?

```
public void mainMenu(BookList b)
{
bool repeat = false;
Console.WriteLine("Welcome to Library.");
Console.WriteLine("What do you want to do?");
Console.WriteLine("1. Add a book.");
Console.WriteLine("2. Delete book.");
Console.WriteLine("3. Borrow a book.");
Console.WriteLine("4. Return book.");
Console.WriteLine("5. See list of available books.");
Console.WriteLine("6. See list of all books.");
Console.WriteLine("7. Exit");
string userChoice = Console.ReadLine();
switch (userChoice)
{
case "1":
do
{
b.addBook();
Console.WriteLine("What do you want to do now?");
Console.WriteLine("Add another book? - Press 1.");
Console.WriteLine("Go back to main menu? - Press any other button.");
ConsoleKeyInfo keyPressed;
keyPressed = Console.ReadKey();
if (keyPressed.Key == ConsoleKey.D1 || keyPressed.Key == ConsoleKey.NumPad1)
{
repeat = true;
Console.Clear();
}
else
{
repeat = false;
}
}
while (repeat == true);
Console.Clear();
this.mainMenu(b);
break;
case "2":
b.deleteBook();
break;
case "3":

Starting with a small nitpick, that the rest of the application code already has -- use full names for variables. Avoid the habit of short variable names like b. Feel free to use the full name of boardList instead. Remember that readability, clarity, and simplicity are key in programming.

Remember to always guard your inputs. We don't know if bookList is null or not and we are attempting to use it by calling a method on it. This will cause a NullReferenceException. Simply check if it is null before using it.

The repeat variable is sticking out to me. It is declared at the top, but only used in one of the case statements. Using a break statement to control the do-while loop would be a cleaner approach.

case "1":
    do
    {
        b.addBook();
        Console.WriteLine("What do you want to do now?");
        Console.WriteLine("Add another book? - Press 1.");
        Console.WriteLine("Go back to main menu? - Press any other button.");
        ConsoleKeyInfo keyPressed;
        keyPressed = Console.ReadKey();
        if (keyPressed.Key != ConsoleKey.D1 && keyPressed.Key != ConsoleKey.NumPad1)
        {
            Console.Clear();
            break;
        }
    } while (true);

    Console.Clear();
    this.mainMenu(b);
    break;

The next issue with the code is the line this.mainMenu(b);. This causes a recursive call that can fill the stack if called many many times (via the input "1" -> "any key but 1" -> repeat) or worse, cause some interesting bugs later on.

Try adding another console output after this.mainMenu(b);, hit the above path a couple of times, and try to figure out what happens.

A way to address this is to remove the line this.mainMenu(b);, make this mainMenu(...) function private and create a different public method that repeats the call to mainMenu(...).

Finally, we can add some helper functions with decent names and enums to help define what case "1": actually means. This helps to eliminate the magic, hard-coded, numbers to some degree (they are just a nature of the beast when dealing with these types of menus).

When we put this all together, we get a different approach. Remember that this is only based on the code I've seen, so take it as an example/another way to do the same thing, not an objectively better solution.

```
// Map user input (0, 1, 2...) to human-readable action names
// The actions should match up with the menuText string.
private enum MainMenuAction : int
{
invalid = 0,
repeat = 1,

addBook = 1,
deleteBook = 2,
// TODO etc.. fill the rest in
exit = 7,
}

// What we want our main menu to say.
// The numbers should match up with the MainMenuAction enum.
private static readonly string menuText =
"Welcome to Library.\n" +
"What do you want to do?\n" +
"1. Add a book.\n" +
"2. Delete a book.\n" +
"TODO etc... fill it out here\n" +
"7. Exit.";

public void mainMenu(BookList bookList)
{
// Make sure bookList is not null, since we will be operating on it.
if (bookList == null)
{
// TODO Handle null bookList
return;
}

// Do our application-level looping here
do
{
// Display the main menu text
Console.WriteLine(menuText);

// Get the user input
int userChoice;
if (tryGetInput(out userChoice))
{
// If user wants to exit, then exit the main menu.
if (userChoice == (int)MainMenuAction.exit)
{
break;
}

// Handle the user input by casting the int to a human-readable name
handleMainMenuAction((MainMenuAction)userChoice, bookList);
}
else
{
// TODO Handle unexpected input (parsing to int failed, user error)
}
} while (true);
}

private void handleMainMenuAction(MainMenuAction action, BookList bookList)
{
// Make sure bookList is not null, since we will be operating on it.
if (bookList == null)
{
// TODO Handle null bookList
return;
}

// Ensure that the enum is defined
if (!Enum.IsDefined(typeof(MainMenuAction), action))
{
// TODO Handle undefined enum (user error)
return;
}

switch (action)
{
case MainMenuAction.addBook:

// Command-level looping
do
{
bookList.addBook();
} while (askUserForRepeat("Add another book"));
break;

case MainMenuAction.deleteBook:
bookList.deleteBook();
break;

// TODO Fill out the other cases

case MainMenuAction.invalid:
default:
// TODO Handle invalid action (internal error -- not yet implmented, probably)
break;
}
}

// Just a wrapped version of int.TryParse that grabs from the command line
private bool tryGetInput(out int input)
{
return int.TryParse(Console.ReadLine(), out input);
}

// Displays 'repeatTex

StackExchange Code Review Q#142627, answer score: 2