Parsing and looping on user Yes/No console input in C#

Question

I'm writing a method that I will use in another code so I'm writing it separately first.

The code returns a boolean based on the first char of the input (yes, Yes, y, yeah, No, ...). But when the input is different from Y or N it starts acting up. In eg3 It stays in the loop until The Console.Read encounters a Y or an N or there are no chars left. In the last case, it will ask again for input.

Am I using the Console.Read wrong or is there another error in my code? Thanks in advance

EDIT: Is the solution, in the end, an efficient one?

eg1:

Yes or No?
YeahIthinkso   <--my input
True

eg2:

Yes or No?
Negative       <--my input
False

eg3:

Yes or No?
Idontknow      <--my input
You can only answer with Yes or No
Yes or No?
You can only answer with Yes or No
Yes or No?
You can only answer with Yes or No
Yes or No?
False

The Code:

static void Main(string[] args)
        {
            char YN = 'x';
            bool ans = false;

            while (YN == 'x')
            {
                Console.WriteLine("Yes or No?");
                YN = char.ToUpper(Convert.ToChar(Console.Read()));


                switch (YN)
                {
                    case 'Y':
                        ans = true;
                        break;

                    case 'N':
                        ans = false;
                        break;

                    default:
                        Console.WriteLine("You can only answer with Yes or No");
                        YN = 'x';
                        break;
                }
            }
            Console.WriteLine(ans);

Alternate solution based on @StuartLC's answer:

bool ans = true, loop = true;
do
{
    switch (Console.ReadLine().ToUpper().FirstOrDefault())
    {
        case 'Y':
            ans = true;
            loop = false;
            break;

        case 'N':
            ans = false;
            loop = false;
            break;

        default:
            Console.WriteLine("You can only answer with Yes or No");
            break;
    }
} while (loop==true);

Console.WriteLine(ans);

You are using `Console.Read` which will read 1 character at a time. If you intend to compare the full input once, use `Console.ReadLine` and read it as a string, then if necessary chop off the first character for comparison. — Lasse V. Karlsen, Jan 02 '20 at 12:23
If you print the failing value of YN along with the error message, you would see that (for input "Idontknow") you get the values "I", "D", "O" before it finds the "N" — Hans Keﬆing, Jan 02 '20 at 12:44

StuartLC · Accepted Answer · 2020-01-02T16:27:08.190

As per @Lasse's comment - if the user types in multiple characters, you'll loop on each character in the string typed in by the user, resulting in the printing a new line of output for each character the user typed. Instead, use of ReadLine will parse the input as a single string, and then the Linq extension FirstOrDefault() will safely obtain the first char in the string:

YN = char.ToUpper(Console.ReadLine().FirstOrDefault());

As an aside, instead of starting a while loop with a forced false condition, C# also supports a do-while loop syntax which fits your requirement better, i.e. at least one iteration through the loop, with a check at the end of the loop:

do
{
    Console.WriteLine("Yes or No?");
    YN = char.ToUpper(Console.ReadLine().FirstOrDefault());

    switch (YN)
    {
        case 'Y':
            ans = true;
            break;

        case 'N':
            ans = false;
            break;

        default:
            Console.WriteLine("You can only answer with Yes or No");
            YN = 'x';
            break;
    }
}
while (YN == 'x');

Re OP's follow up question

Can I now remove the 'YN' completely and put a switch (Console.ReadLine().FirstOrDefault().ToUpper()) in the beginning and a while (ans == true || ans ==false) in the end?

Not quite - since ans is a boolean, it can only have two states (true or false), and you need to model at least 3 states (true, false, and invalid). Although you could use a nullable boolean (bool?) to model the 3 states (where null = invalid), I personally dislike using null to indicate something isn't known, as this invariably leads to the NullReferenceException abyss, and C# hasn't (yet) opted for the "Option" type / Monad (like Java's Optional).

If you're OK with C#8 pattern matching and tuples, you could possibly make the code a bit more concise and refactored as follows, by splitting out the concerns of 'is the input valid' and 'what is the valid input value'?. You could also refactor the switch expression into it's own method to split out the concerns of 'UI' from the parsing logic, which is always a good idea.

static void Main(string[] args)
{
    bool ans;
    bool isValid;
    do
    {
        Console.WriteLine("Yes or No?");
        (ans, isValid) = ParseInput(Console.ReadLine());
        if (!isValid)
        {
            Console.WriteLine("You can only answer with Yes or No");
        }
    }
    while (!isValid);
    Console.WriteLine(ans);

    (bool ans, bool isValid) ParseInput(string input) => 
        char.ToUpper(input.FirstOrDefault()) switch
        {
            'Y' => (true, true),
            'N' => (false, true),
            _ => (default, false)
        };
}

@Jonathan I've updated the answer to address this question. By the way - I was wrong - for some inane reason, `char.ToUpper` [isn't an out-of-the-box extension method](https://stackoverflow.com/q/38548843/314291). You were right. — StuartLC, Jan 02 '20 at 16:20

Parsing and looping on user Yes/No console input in C#

1 Answers1