Stop / correct a do while c#-CodePudding

What i want to do is a code which accepts letters and just stops when a vowel is typed in console. My problem is that yes it does that, but it doesn't stop asking for more letters after the vowel is introduced.

Here is what i have:

using System;
namespace ConsoleApp9
{
    class Program
    {
        static void Main(string[] args)
        {
            char cha;
            
            do
            {
                Console.WriteLine("Insert letter: ");
                cha = char.Parse(Console.ReadLine());

                if (cha == 'a' | cha == 'e' | cha == 'i' | cha == 'o' | cha == 'u')
                {
                    Console.WriteLine("We are sorry, ¡the program ends here!");
                    Console.ReadLine();

                }

            } while (cha != 'a' | cha != 'e' | cha != 'i' | cha != 'o' | cha != 'u') ;

        }
    }
}

CodePudding user response：

Part 1: The Actual bug:

The actual bug in your program is:

while (cha != 'a' | cha != 'e' | cha != 'i' | cha != 'o' | cha != 'u') ;

while (cha != 'a' && cha != 'e' && cha != 'i' && cha != 'o' && cha != 'u') ;

Part 2: Avoid duplicate conditions

In general, I would recommend avoiding duplicated checks. Your current code is checking for vowels twice, which is gives you two opportunities for bugs. As an alternative implementation:

static void Main(string[] args)
{
    char cha;
    
    do
    {
        Console.WriteLine("Insert letter: ");
        cha = char.Parse(Console.ReadLine());

        if (cha == 'a' | cha == 'e' | cha == 'i' | cha == 'o' | cha == 'u')
        {
            Console.WriteLine("We are sorry, ¡the program ends here!");
            Console.ReadLine();
            break;
        }
    } while (true);
}

Here, we use break to exit the loop.

Part 3: Don't use do-while for unconditional loops

Most people prefer unconditional loops to use while rather than do-while, so we can further improve the loop:

while (true)
{
    Console.WriteLine("Insert letter: ");
    char cha = char.Parse(Console.ReadLine());

    if (cha == 'a' | cha == 'e' | cha == 'i' | cha == 'o' | cha == 'u')
    {
        Console.WriteLine("We are sorry, ¡the program ends here!");
        Console.ReadLine();
        break;
    }
}

Notice that I was also able to move the char declaration inside the loop, since we no longer have to worry about scoping concerns.

Part 4: Cleaning up our end condition

At this point, we can go a step further and move the "the program ends here!" functionality out of the loop. In general, you should try to avoid putting code into while loops if the code is going to run exactly once. So, our new code is:

static void Main(string[] args)
{
    while (true)
    {
        Console.WriteLine("Insert letter: ");
        char cha = char.Parse(Console.ReadLine());
        if (cha == 'a' | cha == 'e' | cha == 'i' | cha == 'o' | cha == 'u')
        {
            break;
        }
    }
    Console.WriteLine("We are sorry, ¡the program ends here!");
    Console.ReadLine();
}

Part 5: Other improvements

I'll note that there are numerous little tweaks that might make this code a bit better. I've avoided any such tweaks in favor of making the code conceptually similar to your existing code. We might consider:

Using TryParse, so that the code handles invalid input.
Using "aeiou".Contains to avoid needing 5 equality checks.
Also treat capital letters as vowels (AEIOU).

CodePudding user response：

You are hurting yourself a bit. Use a while loop instead. This template,

int n = 5;
while (  n < 6) 
{
Console.WriteLine("Current value of n is {0}", n);
}

this way it will always check it before executing any input. Alternatively, you could use "break" but that is forcing it.

ideally, you should create a method/function that returns a bool isVowel,

 public static bool IsVowel(this char c)
 {
    long x = (long)(char.ToUpper(c)) - 64;
    if (x*x*x*x*x - 51*x*x*x*x   914*x*x*x - 6894*x*x   20205*x - 14175 == 
        0) 
        return true;
    else 
        return false;
 }

or even directly as shown below.

char c = char.Parse(Console.ReadLine());
bool isVowel = "aeiouAEIOU".IndexOf(c) >= 0;

then you can easily set the true or false part of any loop you wish.