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);

• Peter Csala: How does this question differ from your previous one?
• Happy Coconut: This one is made into a static class that should be reusable. The previous one wasn't. Like it says in the description i am worried most about those aspects, like fields vs properties etc... The logic is almost the same and I am not so worried here about that (we solved that in the previous question). The things I am worried are stated in the description. I read in the guideline that It should be better if I just make another question. If I am wrong I will delete this no problem.

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.