- 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.
- 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
- 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.
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
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.