Is this a good use of goto statement?

Question

I am writing a simple C# console application in which I am solving the problem by using a GoTo statment to return to the label if the input for UserId is not valid.

But I am not sure if I am using the statement correctly. I would like to work around if there is a better way to solve the problem, even without using GoTo statement?

public void CreateAccount()
{
    Console.WriteLine("----- Create New Account -----\n" +
        "Enter following account information:\n");

    getUserID:
    {
        Console.WriteLine("Enter a UserID (Alphanumeric; no special characters): ");
        string userid = Console.ReadLine();
        if (!ValidUserID(userid))
        {
            Console.WriteLine("Userid can only contain A-Z, a-z & 0-9. Try again");
            goto getUserID;
        }
        if (data.IsUserInFile(userid))
        {
            Console.WriteLine("Userid already exists. Try again");
            goto getUserID;
        }
    }
}

I will be using the same approach for other fields such as Pin, AccountType, Balance, AccountStatus etc. so I want to ensure that I am doing this in the right manner before I expand its use.

the best way to use `goto` is, generally, not to. but have you tried a loop instead? — Franz Gleichmann, Apr 14 '21 at 05:39
Why would you use `goto` here when a loop would suffice, and make your intention much clearer? — ProgrammingLlama, Apr 14 '21 at 05:41
@FranzGleichmann I tried with do while loop but as I have two conditions to check, the loop ends when only one condition is satisfied. could you please suggested any approach with loops? — Manaswi Patil, Apr 14 '21 at 05:43
@ManaswiPatil Maybe use an OR boolean operator instead of an AND boolean operator. — Hayden, Apr 14 '21 at 05:44
`if (conditionOne) { } else if (conditionTwo) { } else { break; }`? — ProgrammingLlama, Apr 14 '21 at 05:46
@ManaswiPatil The `else` can only be entered if `!conditionOne && !conditionTwo`, since `conditionOne` and `conditionTwo` will be handled by their own `if` blocks. — ProgrammingLlama, Apr 14 '21 at 05:56
Before the start of the loop, declare a boolean variable `isInvalidUserID` and initialize it to `true`. After that, create a `while`-loop that checks the variable: `while (isInvalidUserID ) { ... }`. Use the current compound statement after the label as the loop's body. (Remove the label of course.) In the loop body, right after the `Console.ReadLine` statement, set `isInvalidUserID` to `false` (to assume that the provided input is valid). Then, inside the `if`-blocks, set `isInvalidUserID` to `true` again, because the input turns out to be invalid. That should do the job. — Bart Hofland, Apr 14 '21 at 06:00
@jason.kaisersmith: _"This question should not be closed."_ -- no, you are wrong about that. If the code works, it's off-topic for Stack Overflow. Stack Overflow isn't a code-review site, as such questions solicit _primarily opinion-based_ answers only. This code should remain closed, and anyone voting to reopen is doing the community a serious disservice. — Peter Duniho, Apr 14 '21 at 06:02
@jason.kaisersmith: _"The only correct way to use the GoTo statement is not to use the GoTo statement!"_ -- you also happen to be wrong about that too. I agree that `goto` shouldn't be used for most branching. But it's essential in a `switch` statement, as it's the only way to allow one `case` to fall through to another. — Peter Duniho, Apr 14 '21 at 06:03
@PeterDuniho . . . I agree about the reasoning why this question is closed. I also think that it would be better to put questions that ask for opinions and alternative ways to implement working code on [Code Review Stack Exchange](https://codereview.stackexchange.com/). — Bart Hofland, Apr 14 '21 at 06:09
@PeterDuniho . . . Anyway, using the `goto` statement inside `switch` statements to jump to other labels would be reasonable, since the scope and the code complexity in which it is used tend to be rather limited. (I personally consider that to be an entirely different statement; I regard it as a `goto case` statement instead of just a `goto` statement.) My personal aversion is towards the common use of `goto` to jump to "random" places in the code. So no `goto` without a subsequent `case` keyword for me. ;) — Bart Hofland, Apr 14 '21 at 06:21
You may want to search for "Go To Statement Considered Harmful" by Edsger Dijkstra. It's an oldie (March 1968) but a goodie. — Flydog57, Apr 14 '21 at 06:22
That was the time the origin of most of current programming languages was laid. But none of the language designers of that time (Ritchie, Wirth, Stroustroup), removed goto completely from their languages, as all of them could see valid uses. — PMF, Apr 14 '21 at 06:28
@PeterDuniho I did prefix my statement with "IMHO = In my humble opinion", so that it is clear that I personally don't like the approach. And in a switch statement then there are other approach which I would prefer. But as indicated with IMHO then this is my preference. — jason.kaisersmith, Apr 14 '21 at 06:36
@PMF . . . I personally think that `goto` and `switch` are still available to aid with migration of legacy code and easing a developer's transfer from other languages to C# and object oriented design. But, of course, where there are multiple ways to accomplish things, there are multiple opinions about good practice as well. That's not bad per-se. It's often based on experience and comfort. — Bart Hofland, Apr 14 '21 at 06:49
@PeterDuniho . . . By the way... I personally prefer to avoid `switch` statements as well, except for *very* simple scenarios. For more complex scenarios, I would personally prefer to use polymorphism (like applying a [strategy pattern](https://en.wikipedia.org/wiki/Strategy_pattern) or a [state pattern](https://en.wikipedia.org/wiki/State_pattern)). So no `switch` statements with `goto case` for me either. ;) — Bart Hofland, Apr 14 '21 at 06:51
@Flydog57 Even the great Dijkstra admitted that `goto` has uses (perhaps not here though!), for example it is often used in tightly looped algorithms to break out of inner loops, and to reorder branching. See also https://stackoverflow.com/questions/46586/goto-still-considered-harmful — Charlieface, Apr 14 '21 at 08:17
goto is fine. Ignore the FUD. Don't give too much credence to Djikstra, he thought greater than or equal to was a difficult symbol to understand and forever broke math in programming for countless languages because of it. — TylerH, Apr 14 '21 at 19:53

score 3 · Accepted Answer · answered Apr 14 '21 at 06:10

A simple approach would be to use an if/else if/else construct:

public void CreateAccount()
{
    Console.WriteLine("----- Create New Account -----\n" +
        "Enter following account information:\n");

    string userid = null;
    while (true)
    {
        Console.WriteLine("Enter a UserID (Alphanumeric; no special characters): ");
        userid = Console.ReadLine();
        if (!ValidUserID(userid))
        {
            Console.WriteLine("Userid can only contain A-Z, a-z & 0-9. Try again");
        }
        // if the condition above isn't matched, we'll check this condition
        else if (data.IsUserInFile(userid))
        {
            Console.WriteLine("Userid already exists. Try again");
        }
        // if that isn't matched either, then we have an acceptable userid and can exit the loop
        else
        {
            break;
        }
    }
}

Note that I've moved the userid declaration out of the loop, since you presumably need to use it after the loop.

Alternatively, you could potentially break up your code a little more. The whole method for reading the userId could be moved out into its own method:

public string ReadUserId()
{
    while(true)
    {
        // Prompt and read answer
        Console.WriteLine("Enter a UserID (Alphanumeric; no special characters): ");
        string userId = Console.ReadLine();
        
        // If the id is valid, return it and leave the method (ending the loop)
        if (ValidUserId(userId))
        {
            return userId;
        }
        
        // If we got here, then the id is invalid and we should inform the user to try again
        Console.WriteLine("Userid can only contain A-Z, a-z & 0-9. Try again");
    }
}

Now in the main method we can reference this method when we need to obtain a userId. The loop for checking if the user already exists can now be implemented separately to reading the user input, so there are no longer concerns about two different conditions:

public void CreateAccount()
{
    Console.WriteLine("----- Create New Account -----\n" +
        "Enter following account information:\n");

    string userId = null;
    bool isNewUser = false;
    while (!isNewUser)
    {       
        // Use our other method to read the user id
        userId = ReadUserId();
    
        if (data.IsUserInFile(userId))
        {
            Console.WriteLine("Userid already exists. Try again");
            continue;
        }
        
        isNewUser = true;
    }
}

I will try this approach. My only concern is as I would be following similar approach for other fields, the code might get confusing. — Manaswi Patil, Apr 14 '21 at 06:31
@ManaswiPatil Separating things out into logically named methods can definitely help make code less confusing. I know it seems like more distributed code will make it more confusing, but consider reading the `CreateAccount()` method in my second example. You see `userId = ReadUserId();` and instantly understand that the method will obtain a user id, etc. so it makes the actual core of the process easier to digest, and then if you need to see what the individual bits do you can go to the individual methods. — ProgrammingLlama, Apr 14 '21 at 06:37
Also note that not everything needs to be in the loop. Once the `while` loop in either of the examples' `CreateAccount` methods is complete, you can use the validated `userId` as necessary. — ProgrammingLlama, Apr 14 '21 at 06:38
@ManaswiPatil To avoid confusion then ensure that you use separate methods. It is also most likely that you can actually make the methods generic and pass in the details you need, such as the name of the field you want to check. — jason.kaisersmith, Apr 14 '21 at 06:38
@Llama your approach has worked well for me. using `continue` with `if` loop in `while` loop is giving the exact working I needed. Thanks. — Manaswi Patil, Apr 15 '21 at 00:26

Is this a good use of goto statement?

1 Answers1