patterncsharpMajor
A completely repetitive console prompt, a completely repetitive console prompt
Viewed 0 times
consolerepetitivepromptcompletely
Problem
So I wrote this for my Mandelbrot Generator, to collect and use input from the user, and I made it dynamic enough to be useful anywhere.
I'm really interested in all critiques of it, as I'm going to be using it a lot now.
The idea is to allow the user to easily show messages on the console, that require input. Once valid input is received, the method returns.
Example usage:
This will prompt the user to enter a value between
Which will only prompt the user for a valid
``
/// An optional delegate to a method which can perform additional validation if the input is of the target type.
/// The input collected from the user when it is deemed valid.
static T Prompt(string message, bool requireValue, T defaultValue = default(T), string failureMessage = null, Func validationMethod = null)
where T : struct
{
if (!requireValue)
Console.Write(string.Format(message + " [{0}]: ", defaultValue));
else
I'm really interested in all critiques of it, as I'm going to be using it a lot now.
The idea is to allow the user to easily show messages on the console, that require input. Once valid input is received, the method returns.
Example usage:
int numberOfCores = Environment.ProcessorCount - 1;
numberOfCores = Prompt($"Enter the number of cores to use (1 to {Environment.ProcessorCount})",
false,
numberOfCores,
$"The value must be between 1 and {Environment.ProcessorCount}",
delegate (int x) { return x >= 1 && x <= Environment.ProcessorCount; });This will prompt the user to enter a value between
1 and Environment.ProcessorCount. But, you could also use:ushort maxIterations = 1000;
maxIterations = Prompt("Enter the maximum number of iterations", false, maxIterations);Which will only prompt the user for a valid
ushort value.``
///
/// This will repeatedly prompt the user with a message and request input, then return said input (if valid).
///
/// The type of input that should be returned.
/// The message to initally display to the user.
/// Whether or not to allow use of a defaultValue.
/// The default value to be returned if a user enters an empty line ("").
/// The message to display on a failure. If null, then the message` parameter will be displayed on failure./// An optional delegate to a method which can perform additional validation if the input is of the target type.
/// The input collected from the user when it is deemed valid.
static T Prompt(string message, bool requireValue, T defaultValue = default(T), string failureMessage = null, Func validationMethod = null)
where T : struct
{
if (!requireValue)
Console.Write(string.Format(message + " [{0}]: ", defaultValue));
else
Solution
String Formatting
This innocent code will break if
You're already using interpolated strings then you may use them here too:
Now you'll also see you repeat same code inside your loop, we're lazy so we like to avoid repeated code, move it inside your
Note I also dropped exit condition, it'll be directly handled in your code with a
Input Conversion
It's time to read input. Most obvious issue is your conversion function
Simply replace it with:
Let's write an helper function (compacted code for brevity):
Use it:
Validation
Your validation code is also little bit too convoluted, it may be simplified:
Result
Everything all together:
Further Refinements
So far we worked on function implementation however also its interface may be improved. First of all you're using a boolean parameter, especially when arguments list is long you may want to use an enumeration:
Moreover your function accepts many arguments (and most of them have default value). If you like to keep default value in-place I'd suggest to add more overloads (for most common cases). Note that you may also assume that if there is a default value then requireValue is false (to provide two simplified overloads).
Last change is generic constraint you applied. Limiting
Moreover it also exclude strings (and many user inputs are plain text). Change that constraint to
if (!requireValue)
Console.Write(string.Format(message + " [{0}]: ", defaultValue));
else
Console.Write(message + ": ");This innocent code will break if
message contains any String.Format() formatting code. It may be acceptable if it's a private function but to make it reusable you have to address this issue, you have to treat callers input as user input. It's even slightly faster but you shouldn't really care about this here:Console.Write(string.Format("{0} [{1}]: ", message, defaultValue));You're already using interpolated strings then you may use them here too:
Console.Write($"{message} [{defaultValue}]");Now you'll also see you repeat same code inside your loop, we're lazy so we like to avoid repeated code, move it inside your
while loop, only at its beginning. About your loop: it has to be executed at least once then do/while is more clear (about its intent) than while.do
{
if (requireValue) ...
} while (true);Note I also dropped exit condition, it'll be directly handled in your code with a
return statement.Input Conversion
It's time to read input. Most obvious issue is your conversion function
Retrieve(). It's prolix and it's not even complete (what about decimal and char, for example?).Simply replace it with:
var result = Convert.ChangeType(line, typeof(T));Let's write an helper function (compacted code for brevity):
bool TryConvert(string text, bool ignoreIfEmpty, ref T value)
{
// null is not possible, if it happens we may want ChangeType()
// to throw ArgumentNullException because it's an actual error...
if (ignoreIfEmpty && String.IsNullOrWhiteSpace(text))
return true;
try
{
value = (T)Convert.ChangeType(text, typeof(T));
}
catch (InvalidCastException) { return false; }
catch (FormatException) { return false; }
catch (OverflowException) { return false; }
return true;
}Use it:
T value = defaultValue;
if (!TryConvert(Console.ReadLine(), !requireValue, out value)
{
Console.WriteLine("Input is not valid.");
continue;
}Validation
Your validation code is also little bit too convoluted, it may be simplified:
if (validationMethod != null && !validationMethod(value))
{
Console.WriteLine(failureMessage ?? "Input is not valid.");
continue;
}Result
Everything all together:
static T Prompt(string message, ...
{
do
{
Console.Write(requireValue ? $"{message} [{defaultValue}]: " : $"{message}: ");
T value = defaultValue;
if (!TryConvert(Console.ReadLine(), !requireValue, out value)
{
Console.WriteLine("Input is not valid.");
continue;
}
if (validationMethod != null && !validationMethod(value))
{
Console.WriteLine(failureMessage ?? "Input is not valid.");
continue;
}
return value;
} while (true);
}Further Refinements
So far we worked on function implementation however also its interface may be improved. First of all you're using a boolean parameter, especially when arguments list is long you may want to use an enumeration:
var maxIterations = Prompt("Number of iterations", PromptOptions.Required, 1000);Moreover your function accepts many arguments (and most of them have default value). If you like to keep default value in-place I'd suggest to add more overloads (for most common cases). Note that you may also assume that if there is a default value then requireValue is false (to provide two simplified overloads).
Last change is generic constraint you applied. Limiting
T to struct doesn't stop user of your function to use user defined value types (you don't know how to manage):struct Point { public int X; public int Y }
var result = Prompt(...Moreover it also exclude strings (and many user inputs are plain text). Change that constraint to
IConvertible, Convert.ChangeType() will use it and your function may accept any type that can be convertible from string (you may even drop constraint and leave converter to do all its dirty games with all supported esoteric conversions, just write what you expect in your function interface documentation for T).Code Snippets
if (!requireValue)
Console.Write(string.Format(message + " [{0}]: ", defaultValue));
else
Console.Write(message + ": ");Console.Write(string.Format("{0} [{1}]: ", message, defaultValue));Console.Write($"{message} [{defaultValue}]");do
{
if (requireValue) ...
} while (true);var result = Convert.ChangeType(line, typeof(T));Context
StackExchange Code Review Q#104184, answer score: 20
Revisions (0)
No revisions yet.