Console autocomplete for complex names

Question

I am currently writing a C# console application. Part of it is, that the user needs to enter a fairly complex system name. To make it easier I wrote a function that uses a string[] keywords and auto completes the string the user is typing in - on the run (which makes it different to this post in my opinion)

The code is working and acts as expected, but I am curious how the code could be improved (e.g. usability, efficiency). Also, what functionalities are missing, which you'd expect?

Thank you for the feedback!

    class Program
    {
        static void Main(string[] args)
        {
            string[] Keywords = new string[4] { "aX1223", "aE_334", "L_test1", "L_test2" };
            if (Keywords.Length == 0)
                throw new ArgumentException(nameof(Keywords), "No Keywords set!");

            bool searching = true;                 // true while looking for the keyword
            Console.CursorVisible = true;                 // To help users understand where they are typing
            string userInput = "";                   // Initialization of output
            string suggestion = Keywords[0];          // Initialization of default suggestion
            int toClear = suggestion.Length;    // Get the length of the line that needs to be cleared

            while (searching)
            {
                Console.Write(new String(' ', toClear));    // Clear line
                Console.CursorLeft = 0;                     // Relocate cursor to line start
                Console.Write(userInput);                      // Write what has been written previously

                if (suggestion != "")                        // If there is a suggestion fitting the entered string,
                {                                           // complete the string in color and reset the cursor
                    int col = Console.CursorLeft;
                    Console.ForegroundColor = ConsoleColor.Magenta;
                    Console.Write(suggestion.Substring(userInput.Length));
                    Console.ForegroundColor = ConsoleColor.White;
                    Console.CursorLeft = col;
                }

                string tempInput = Console.ReadKey().KeyChar.ToString();

                if (tempInput.Equals("\r"))                 // Evaluate input:
                {                                           //  -> Enter
                    if (!suggestion.Equals(""))             //     Valid entry?
                    {
                        searching = false;
                        userInput = suggestion;           //      -> use suggestion
                    }
                }
                else if (tempInput.Equals("\b"))            // Deleting last sign
                {
                    if (userInput.Length > 0)
                        userInput = userInput.Substring(0, userInput.Length - 1);
                }
                else                                        // Any other sign is added to the string
                    userInput += tempInput;

                // Set length to clear, if suggestion == "", the system string length needs to be cleared
                toClear = (userInput.Length > suggestion.Length) ? userInput.Length : suggestion.Length;

                // Reset suggestion. If no match is found, suggestion remains empty
                suggestion = "";

                // Check keywords for suggestion
                for (int i = 0; i < Keywords.Length; i++)
                {
                    if (Keywords[i].StartsWith(userInput))
                    {
                        suggestion = Keywords[i];
                        break;
                    }
                }
            }

            // reset cursor visibility
            Console.CursorVisible = false;
        }
    }

Answer 1

A few minor comments:

• Don't mix equality types.

  You use suggestion != "" in one place, then !suggestion.Equals(""). Don't. Use the same type of equality (either using .Equals or not using .Equals) everywhere.

• Only use non-braced statements in the case of guard clauses, and even then be careful.

  Basically, this is OK:

  if (Keywords.Length == 0)
      throw new ArgumentException(nameof(Keywords), "No Keywords set!");
  

  This is not:

  if (userInput.Length > 0)
      userInput = userInput.Substring(0, userInput.Length - 1);
  

• Try to use built-ins (especially System.Math) when possible.

  I.e. take toClear = (userInput.Length > suggestion.Length) ? userInput.Length : suggestion.Length; and convert it to toClear = Math.Max(userInput.Length, suggestion.Length). They're functionally equivalent and the Math.Max reads nicer.

The rest of it looks good, I think you did a pretty good job implementing the idea here. I might suggest building functions out, but this is a small enough program it's probably not necessary.

Answer 2

Comments

There's a common misconception about comments. You should not write comments to explain what something does. Your code should be clear enough so that does comments are not necessary. Sadly, there are times where we need to write complex code that is hard to understand. In this very case, you are "allowed" to write comments to explain :

1- Why you did what you did. Example :

FooBar was super complex because of Bar and Foo dependency, so I had to write this like this because of that.

This comment gives insight on what happened in your head while you were writing the code. So if someone ever comes up on the FooBar complex method, they'll be able to use your comment to understand what needs to be considered.

2- Explain what the code does.

FooBar does something by using [Insert shady algorithm] to have maximum performance.

A good rule of thumb would be to ask yourself "If I came back to my code in 2 weeks, is there something I might have trouble understanding in my code?". If the answer is yes, try to refactor that thing. If the answer is still yes, add a comment.

So, in my opinion, most, if not all, your comments could be deleted.

Coding style

Brackets

I'll use this piece of code as an example :

else if (tempInput.Equals("\b"))
{
    if (userInput.Length > 0)
        userInput = userInput.Substring(0, userInput.Length - 1);
}
else
    userInput += tempInput;

The parenthesis on the else if aren't even necessary, yet you use them, but with the else, you don't. This lack of consistency makes the code harder to read and adds bugs potential. Since the Apple bracket's fail, I always use brackets around my flow operators (for, if, etc..). You should too!

Constants vs local variables

Keywords could be a const. That's that. :p

Application flow

Method extraction

There is a lot of different flows in your code (lots of ifs, while, for, etc), it would help if some parts were separated in different methods. For example :

private string GetSuggestionFromKeywords(string userInput)
{
    for (int i = 0; i < Keywords.Length; i++)
    {
        if (Keywords[i].StartsWith(userInput))
        {
            return Keywords[i];
        }
    }
}

This method is small, super easy to understand and as a bonus you get a method name which "acts as a comment" on what the inside of the method does.

Tip : Try to figure out what other pieces of code could be extracted to methods, it's a very good exercise to help you develop (pun intended) as a better developer.

Conditions

else if (tempInput.Equals("\b"))
{
    if (userInput.Length > 0)
        userInput = userInput.Substring(0, userInput.Length - 1);
}

So, what if tempInput.Equals("\b") == true but userInput.Length > 0 == false? Nothing happens right? In this case, why not use :

else if (tempInput.Equals("\b") && userInput.Length > 0)
{
    userInput = userInput.Substring(0, userInput.Length - 1);
}

That's one if less. Try to reduce nesting as much as possible as it cripples readability.

Tip : Once again, try to find every way you can to reduce nesting, this is another good exercise which will make you a better developer.

Exceptions

if (Keywords.Length == 0)
    throw new ArgumentException(nameof(Keywords), "No Keywords set!");

Now, you're the one who sets the Keywords and they are not modifiable by anyone, ever, except you. This means that the only way this exception would be thrown is if you forgot to write the line just above, which would be hard, since you wrote it already.

Considering this, you should think about removing this exception.

Regarding my "tips". Extracting methods, reducing nesting etc. should never be at the detriment of code readability. If extracting a method makes your code uglier, don't do it. But think about ways to do it to make your code more readable.