20
\$\begingroup\$

Yesterday I stumbled upon a Stack Overflow question that asked if it was possible to implement Tab-triggered auto-completion in a console application.

I thought that was an interesting idea, so I went ahead and wrote a small program that does just that, given a hard-coded string[] - I might eventually refactor it into its own class and reuse it in my own projects if I ever actually need auto-completion in a console app, but before I do that I'd like some feedback on the way it's implemented, given static aside, the logic itself is pretty much the way I'll have it regardless of whether it's in a dedicated class as part of something much bigger, or right there in a console application that does nothing but verify that the code works.

Pressing Tab when there's no input will change the current line to say Bar; because there's a word in data that starts with Bar, pressing Tab again will autocomplete to Barbec; another Tab will make it Barbecue, and then any subsequent Tab will have no effect, because nothing in the data starts with Barbecue - but then you could Backspace until the input is Ba, type a t to make it Bat, and when you press Tab then it autocompletes to Batman.

In other words, it all works exactly as it should. But does it look right?

class Program
{
    static void Main(string[] args)
    {
        var data = new[]
        {
            "Bar",
            "Barbec",
            "Barbecue",
            "Batman",
        };

        var builder = new StringBuilder();
        var input = Console.ReadKey(intercept:true);

        while (input.Key != ConsoleKey.Enter)
        {
            if (input.Key == ConsoleKey.Tab)
            {
                HandleTabInput(builder, data);
            }
            else
            {
                HandleKeyInput(builder, data, input);
            }

            input = Console.ReadKey(intercept:true);
        }
        Console.Write(input.KeyChar);
    }

    /// <remarks>
    /// https://stackoverflow.com/a/8946847/1188513
    /// </remarks>>
    private static void ClearCurrentLine()
    {
        var currentLine = Console.CursorTop;
        Console.SetCursorPosition(0, Console.CursorTop);
        Console.Write(new string(' ', Console.WindowWidth));
        Console.SetCursorPosition(0, currentLine);
    }

    private static void HandleTabInput(StringBuilder builder, IEnumerable<string> data)
    {
        var currentInput = builder.ToString();
        var match = data.FirstOrDefault(item => item != currentInput && item.StartsWith(currentInput, true, CultureInfo.InvariantCulture));
        if (string.IsNullOrEmpty(match))
        {
            return;
        }

        ClearCurrentLine();
        builder.Clear();

        Console.Write(match);
        builder.Append(match);
    }

    private static void HandleKeyInput(StringBuilder builder, IEnumerable<string> data, ConsoleKeyInfo input)
    {
        var currentInput = builder.ToString();
        if (input.Key == ConsoleKey.Backspace && currentInput.Length > 0)
        {
            builder.Remove(builder.Length - 1, 1);
            ClearCurrentLine();

            currentInput = currentInput.Remove(currentInput.Length - 1);
            Console.Write(currentInput);
        }
        else
        {
            var key = input.KeyChar;
            builder.Append(key);
            Console.Write(key);
        }
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ very interesting question \$\endgroup\$
    – Tolani
    Aug 20 '16 at 21:22
3
\$\begingroup\$

Testing

  • Personally I think it’s conceptually easier to test this code if you are returning a string – rather than a void: what if you decided to change the out put device from a console window to your printer – it would not be easy to change that. What if sometimes you wanted a console window and other times you wanted to send the same thing via text message? I guess it’s easier to separate what is being done vs how it is being done. E.g. Output key is the abstract concept but this could be done a number of different ways: (i) to the console, or (ii) through a voice synthesiser etc. I wrote some tests but then i threw it once it broke when i changed the API.

Naming

  • What is data? It’s harder to think of a more abstract name. I’ve changed this to keywords.
  • Personally I like to see the type along with the variable declaration.

Refactoring the code:

  • I simplified and extracted some methods.
  • The The HandleKeyInput and the HandleTabInput both seem to be doing conceptually similar things: a key is inputted and then a result is obtained. If it’s a tab then do X, but if it’s a key then do Y. There’s a common abstraction here that can be extracted.
  • What I want to do is to make the two methods: HandleTabInput and HandleKeyInput to look and be absolutely identical.
  • Data (renamed to keywords) can be a field so there’s no need to pass it in as a variable.
  • I want to make two of your methods to basically be the same. They’re kinda doing this same things. After some refactoring this is what I got:

    public void HandleTabInput(StringBuilder builder, ConsoleKeyInfo keyInput)
    {
            // Perform calculation
        string match = ExtractMatch(builder);
    
            // Alter the builder
        builder.Clear();
        builder.Append(match);
    
            // Print Results
        PrintMatch(match);
    }
    

And then this:

    private void HandleKeyInput(StringBuilder builder, ConsoleKeyInfo keyInput)
    {
        if (keyInput.Key == ConsoleKey.Backspace && builder.ToString().Length > 0)
        {
                // Perform Calculation (nothing here)

                // Alter the builder
            builder.Remove(builder.Length - 1, 1);

                // Print Results
            ClearCurrentLine();                
            Console.Write(builder.ToString().Remove(builder.ToString().Length - 1));
        }
        else
        {
                // Perform calculation (nothing here)

                // Alter the Builder
            var key = keyInput.KeyChar;
            builder.Append(key);

                // Print Reuslts
            Console.Write(key);
        }
    }

We can abstract out the calculation, and then the printing. After some work this was the result. It’s starting to look pretty similar:

    private void HandleKeyInput(StringBuilder builder, ConsoleKeyInfo keyInput)
    {
        if (keyInput.Key == ConsoleKey.Backspace && builder.ToString().Length > 0)
        {
            Program.KeyInput backSpaceKey = new Program.KeyInput.BackspaceInput(builder, keywords);
            backSpaceKey.AlterBuilder();
            backSpaceKey.PrintResult();                
        }
        else
        {
            KeyInput input = new KeyInput.StandardKeyInput(builder, keywords, keyInput.KeyChar);
            input.AlterBuilder();
            input.PrintResult();
        }
    }

You could probably tidy things up a little more.

The final Result:

    public void RunProgram()
    {
        StringBuilder builder = new StringBuilder();
        ConsoleKeyInfo capturedCharacter = Console.ReadKey(intercept: true);

        while (EnterIsNotThe(capturedCharacter))
        {
            KeyInput key = KeyInput.GetKey(builder, keywords, capturedCharacter);
            builder = key.UpdateBuilder();
            key.Print();

            capturedCharacter = Console.ReadKey(intercept: true);
        }

        Console.Write(capturedCharacter.KeyChar);
    }

Concluding Thoughts

  • It reads marginally better. Still If you want to reuse the code you’ll have to make changes, but at least it will be a little easier to do when the time comes.
  • Encapsulation could be a little better – typically speaking you want the KeyInput class to be instantiated via the factory only.

A link to see the full code.

\$\endgroup\$
2
  • \$\begingroup\$ I disagree on your concluding thoughts. If I understand them and the theory behind, in this practical case your code is harder to read read than his. The simpler the better. \$\endgroup\$ Mar 25 '19 at 6:11
  • \$\begingroup\$ @Alexis Pautrot - appreciate your comment. Chrs \$\endgroup\$
    – BenKoshy
    Mar 25 '19 at 18:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.