Choose item from list

Question

I wrote small console application where you can choose an item from given list of strings. Works fine so far but since I am beginner I have some questions if I am doing this right. My biggest concerns are if I should use fields here or properties, if I am using the static keyword correctly, if I should use the fields in the methods directly (like I am doing it now) or I should pass them as parameters. In future this class I want to use in multiple places so it makes sense to be static I think. If you have suggestions on some other parts also please comment.

ConsoleSelectMenu.cs:

namespace ConsoleApp
{
    class ConsoleSelectMenu
    {
        static int selectedLineIndex = 0;
        static ConsoleKey pressedKey;
        static bool readyToSelectUp = true;
        static bool readyToSelectDown = true;
        static bool haveWeStarted = false;

        public static string DrawSelectMenu(SelectMenuOptions options)
        {
            Console.WriteLine(options.Question);
            do
            {
                UpdateMenu(options.Choices);
                pressedKey = Console.ReadKey().Key;
                Console.Write("\b \b");

                readyToSelectUp = pressedKey == ConsoleKey.DownArrow && selectedLineIndex + 1 < options.Choices.Count;
                readyToSelectDown = pressedKey == ConsoleKey.UpArrow && selectedLineIndex - 1 >= 0;

                if (readyToSelectUp) selectedLineIndex++;
                else if (readyToSelectDown) selectedLineIndex--;

            } while (pressedKey != ConsoleKey.Enter);
            return options.Choices[selectedLineIndex];
        }

        private static void UpdateMenu(List<string> list)
        {
            if (!readyToSelectUp && !readyToSelectDown) return;
            if (haveWeStarted) ClearList(list);

            foreach (var item in list)
            {
                bool isSelected = item == list[selectedLineIndex];
                Console.WriteLine($"{(isSelected ? "> " : "  ")}{item}");
            }
            haveWeStarted = true;
        }

        private static void ClearList(List<string> list)
        {
            for (int i = 0; i < list.Count; i++)
            {
                int currentLineCursor = Console.CursorTop;
                Console.SetCursorPosition(0, Console.CursorTop - (i + 1));
                Console.Write(new string(' ', Console.WindowWidth));
                Console.SetCursorPosition(0, currentLineCursor);
            }
            Console.SetCursorPosition(0, Console.CursorTop - list.Count);
        }
    }

    public class SelectMenuOptions
    {
        public string Question { get; set; }
        public List<string> Choices { get; set; }
    }
}

Programs.cs

var options = new SelectMenuOptions
{
    Question = "Please choose an option",
    Choices = new List<string>() { "Opt_1", "Opt_2", "Opt_3" }
};

var result = ConsoleSelectMenu.DrawSelectMenu(options);
Console.WriteLine(result);

Answer 1

Firstly, about fields vs. properties: properties are methods in disguise - they allow you to use logic when changing a variable. Personally, I would almost always use the standard Property property { get; set; } so it's faster to customize the way to read and update a property - unless we want the property to be private.

Secondly, and more importantly, I wouldn't make DrawSelectMenu a static class. The reason for that is when you make a class static, you no longer get the benefits of abstraction, and Dependency Injection. Instead, I would create the following abstract class:

internal abstract class SelectMenu
{
    public string Title { get; set; } = "";
    public List<Option> Options { get; set; } = new List<Option>();
    public Option? SelectedOption { get; set; }

    public SelectMenu(string title, List<Option> options)
    {
        this.Title = title;
        this.Options = options;
    }
    public abstract void Display();

    public void SelectPrevious()
    {
        if (this.SelectedOption != null)
        {
            int indexOfSelected = this.Options.IndexOf(this.SelectedOption);
            if (indexOfSelected != -1)
                this.SelectedOption = this.Options.ElementAt(Modulus((indexOfSelected - 1), this.Options.Count));
        }
    }
    public void SelectNext()
    {
        if (this.SelectedOption != null)
        {
            int indexOfSelected = this.Options.IndexOf(this.SelectedOption);
            if (indexOfSelected != -1)
                this.SelectedOption = this.Options.ElementAt(Mod((indexOfSelected + 1), this.Options.Count));
        }
    }
    private static int Modulus(int a, int b)
    {
        return (Math.Abs(a * b) + a) % b;
    }
}

And then, I would create the following derived class:

internal class ConsoleSelectMenu : SelectMenu
{
    public ConsoleSelectMenu(string title, List<Option> options) : base(title, options)
    {
        this.SelectedOption = this.Options[0];
    }

    public override void Display()
    {
        Console.Clear();
        this.Options.ForEach((option) => Console.WriteLine(this.GetPrefix(option) + option.Name));
    }

    private string GetPrefix(Option option)
    {
        return this.SelectedOption == option ? "> " : "  ";
    }
}

And so Program.cs main function becomes:

var options = new List<Option>() {
    new Option { Name = "opt 1" },
    new Option { Name = "opt 2" },
    new Option { Name = "opt 3" }
};

string title = "Please choose an option";

SelectMenu selectMenu = new ConsoleSelectMenu(title, options);

while (true)
{
    selectMenu.Display();

    var key = Console.ReadKey().Key;

    if (key == ConsoleKey.UpArrow)
        selectMenu.SelectPrevious();
    else if (key == ConsoleKey.DownArrow)
        selectMenu.SelectNext();
    else if (key == ConsoleKey.Enter)
        break;
}

Console.WriteLine(selectMenu?.SelectedOption?.Name);

And that solution allows flexibility; if we'd want, for example, a WindowSelectMenu, we could simply create a class that implements the required methods, and just replace the type of SelectMenu when we create it: SelectMenu selectMenu = WindowSelectMenu(title, options)

We can also refactor the user input handling, and create a generic system for handling input and dispatching events (for example, an OnEnterKeyPressed event).

Thirdly, you can still use a class even if it's not static; you just need to pass a reference to the instance or create a new one. For example:

public Option petOwnershipSurvey()
{
    SelectMenu selectMenu = ConsoleSelectMenu("Do you own a pet", new List<Option>() {
        new Option { Name = "Yes" },
        new Option { Name = "No" },
        new Option { Name = "I don't know" }});
    selectMenu.Display();
    /* insert here a code that gets input from the user */
    
    return selectMenu.SelectedOption;
}

• here's the Option class I created:

internal class Option
{
    public string Name { get; set; } = "";
    public string Description { get; set; } = "";
}

• Also, please note that I haven't addressed all edge cases in my code in order to focus on what's relevant to the question.