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);
}
}
}
-
1\$\begingroup\$ very interesting question \$\endgroup\$Tolani– Tolani2016年08月20日 21:22:19 +00:00Commented Aug 20, 2016 at 21:22
1 Answer 1
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 tokeywords
. - 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.
-
\$\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\$Alexis Pautrot– Alexis Pautrot2019年03月25日 06:11:45 +00:00Commented Mar 25, 2019 at 6:11
-
\$\begingroup\$ @Alexis Pautrot - appreciate your comment. Chrs \$\endgroup\$BenKoshy– BenKoshy2019年03月25日 18:03:27 +00:00Commented Mar 25, 2019 at 18:03