Search code examples
c#if-statementgoto

How should I make my "goto" not to run at the end?


Hello! :D So, I have this code below. It is working but I would not say that it works the way I want it to work. So, I used goto to jump to the end of the code. But at the last stage of if I have a goto that navigates to igen: Console.WriteLine("Remek! Estre 8-ra érted megyek!");. When this case happens, I do not want the code below ( nem: Console.WriteLine("Kár, pedig szívesen elhívtalak volna randizni. :(");) to run. How can I get this to work? Thank you in advance!

using System.Reflection.Emit;

namespace Randi
{
    class Program
    {

        static void Main(string[] args)
        {
            Console.WriteLine("Hány centi vagy?");

            int cm = int.Parse(Console.ReadLine());
            if (cm <= 175 && cm >= 150);
            else goto nem;
            {
                Console.WriteLine("Naa, tök jó! Az alacsony lányok cukik.");
                Console.WriteLine("Hány éves vagy?");
                int év = int.Parse(Console.ReadLine());
                if (év <= 18 && év >= 16);
                else goto nem;
                {
                    Console.WriteLine("Akkor még korban is összepasszolunk!");
                    Console.WriteLine("Van barátod?");
                    
                    string kapcsolat = Console.ReadLine();
                    if (kapcsolat.Contains("Nincs"));
                    else goto nem;
                    {
                        Console.WriteLine("Örülök neki! Eljönnél velem randizni? (Igen/Nem)");

                        string randi = Console.ReadLine();
                        if (randi.Contains("Igen")) goto igen;
                        else goto nem;
                    } 
                        
                }
            
            }
        igen:
            Console.WriteLine("Remek! Estre 8-ra érted megyek!");
        nem:
            Console.WriteLine("Kár, pedig szívesen elhívtalak volna randizni. :(");

            Console.ReadKey();
        }

    }
}

Solution

  • You can just jump from the second-to-last case to the end:

    // snip!
    igen:
        Console.WriteLine("Remek! Estre 8-ra érted megyek!");
        goto end;
    nem:
        Console.WriteLine("Kár, pedig szívesen elhívtalak volna randizni. :(");
    
    end:
        Console.ReadKey();
    

    However, as anyone who ever coded would point out, using goto for complex control flow is a bad idea in general, in case you have a really good justification.

    You seem to have some logic that determines one of two possible outcomes, nem and igen (I don't know your language so I have no idea what this code actually does, but alas) based on a user interaction. We can refactor the part that determines the outcome into a method and separate it from the code that acts upon that outcome:

    static void Main()
    {
        var outcome = Interact();
    
        if (outcome)
        {
            Console.WriteLine("Remek! Estre 8-ra érted megyek!");
        }
        else
        {
            Console.WriteLine("Kár, pedig szívesen elhívtalak volna randizni. :(");
        }
    
        Console.ReadKey();
    }
    
    static bool Interact()
    {
        int cm = int.Parse(Console.ReadLine());
        if (cm <= 175 && cm >= 150);
        else return false;
        {
            Console.WriteLine("Naa, tök jó! Az alacsony lányok cukik.");
            Console.WriteLine("Hány éves vagy?");
            int év = int.Parse(Console.ReadLine());
            if (év <= 18 && év >= 16);
            else return false;
            {
                Console.WriteLine("Akkor még korban is összepasszolunk!");
                Console.WriteLine("Van barátod?");
                
                string kapcsolat = Console.ReadLine();
                if (kapcsolat.Contains("Nincs"));
                else return false;
                {
                    Console.WriteLine("Örülök neki! Eljönnél velem randizni? (Igen/Nem)");
    
                    string randi = Console.ReadLine();
                    if (randi.Contains("Igen")) return true;
                    else return false;
                }
            }
        }
    }
    

    All I did was replace the goto nem with return false and goto igen with return true. This is a bit better, but tracking the flow of this code is still very hard. First of all, every if (cond); else return x is a very confusing structure. It's equivalent and more understandable to say if (!cond) return x; Also, you're using blocks of code, but they don't introduce any useful structure, only increase nesting and make it harder to follow. We can just get rid of all the additional braces and tabs. Interact now becomes:

    static bool Interact()
    {
        int cm = int.Parse(Console.ReadLine());
    
        if (cm > 175 || cm < 150)
        {
            return false;
        }
    
        Console.WriteLine("Naa, tök jó! Az alacsony lányok cukik.");
        Console.WriteLine("Hány éves vagy?");
    
        int év = int.Parse(Console.ReadLine());
    
        if (év > 18 || év < 16)
        {
            return false;
        }
        
        Console.WriteLine("Akkor még korban is összepasszolunk!");
        Console.WriteLine("Van barátod?");
    
        string kapcsolat = Console.ReadLine();
    
        if (!kapcsolat.Contains("Nincs"))
        {
            return false;
        }
    
        Console.WriteLine("Örülök neki! Eljönnél velem randizni? (Igen/Nem)");
    
        string randi = Console.ReadLine();
        
        if (!randi.Contains("Igen"))
        {
            return false;
        }
    
        return true;
    }
    

    This looks nicer. At least now you can read the method top down and it's easy to see that what we do is read user input 3 times, check some conditions on that input and determine whether it's valid or not.

    I'd say this is enough for your toy example, but after this pass more opportunities for refactoring present themselves - the method effectively handles 3 independent conditions, so maybe 3 independent methods for each validation step would make the code more readable? By removing gotos you get much more control over the structure of the code, and structure is key to readability. Honestly, the nesting levels and gotos from your original snippet make me very tired by just looking at them. You'd quickly find that working with such code is a nuisance, and you don't want programming to be a nuisance.