Search code examples
c#if-statementnested-loops

cleaning up super gross arrow head anti-pattern in c#


I'm fairly new to coding and I'm looking to get rid of bad habits early and start writing clean and efficient code. I am working on a console application that references an API and i have a series of deeply nested 'if's (at points up to 10 levels deep!).

commonLogic forQuote = new commonLogic();

if (countryRes == CountryDes)
{
    //staying in country
    try2:
    //display reasons for travel
    Console.WriteLine("What Best Describes Your Reason For Traveling?");
    Console.WriteLine(" ");
    Console.WriteLine("1. United States Resident traveling Inside the U.S.");
    Console.WriteLine("2. Visiting United States For Business or Pleasure.");
    Console.WriteLine("3. Immigrating to The Unites States.");
    Console.WriteLine("4. Student, Faculty Member or Scholar With a J-1, F-1, H-3, M-1, or Q-1 Visa.");
    Console.WriteLine(" ");
    var x = Console.ReadLine();
    Console.Clear();

    if (x == "1")
    {
        //US resident
        //first print
        forQuote.gatherUserData();
    }
    else if (x == "2")
    {
        try3:
        //visiting the US
        Console.WriteLine("What Type of Coverage Do You Need?");
        Console.WriteLine(" "); 
        Console.WriteLine("1. Medical voerage");
        Console.WriteLine("2. Trip Cancellation");
        var r = Console.ReadLine();
        Console.WriteLine(" ");
        Console.Clear();

        if (r == "1")
        {
            //medical coverage
            Console.WriteLine("What Type of Medical Coverage Do You Want?");
            Console.WriteLine(" ");
            Console.WriteLine("1. Scheduled benifits");
            Console.WriteLine("2. Comprehensive Benifits");
            var s = Console.ReadLine();
            Console.WriteLine(" ");
            Console.Clear();

            if (s == "1")
            {
                //second print
                forQuote.gatherUserData();
            }
            else if (s == "2")
            {
                //comprehensive benifits
                //third print
                forQuote.gatherUserData();
            }
            else
            {
                //first else
                Console.WriteLine("Invalid Input. Please Try Again");
            }
        }
        else if (r == "2")
        {
            //trip canccelation
            //fourth print
            forQuote.gatherUserData();
        }
        else
        {
            //secondelse
            Console.WriteLine("Invalid Input. Please Try Again");
            goto try3;
        }
    }
    else if (x == "3")
    {
        //immigration
        //fithprint
        forQuote.gatherUserData();
    }
    else if (x == "4")
    {
        //students...
        //sixthprint
        forQuote.gatherUserData();
    }
    else
    {
        //thirdelse
        Console.WriteLine("Invalid Input. Please try Again");
        goto try2;
    }

}

This is only a small sample of this gross nest of ifs. I have done a lot of research on cleaning this up and am having a hard time understanding/using the answers I found. The biggest problem I have had with refactoring has been that each following if is directly reliant on the if before it.

I also made a logic table of what input you need to reach each if. I'll drop it in here in case it's helpful: Excel table showing if paths

I would really appreciate some help, and an explanation of why your answer improves readability and efficiency would be excellent as well.


Solution

  • Look at this code:

    bool loop = true;
    
    while(loop)
    {
        Console.WriteLine("Question");
        Console.WriteLine("1. Ans1");
        Console.WriteLine("2. Ans2");
        Console.WriteLine("3. Exit");
        string resp = Console.ReadLine();
    
        switch(resp)
        {
            case "1":
            Console.WriteLine("Ans1 chosen");
            break;
            case "2":
            SomeQuestion();
            break;
            case "3":
            loop = false;
            break;
            default:
            Console.WriteLine("Invalid Input. Please try Again");
            break;        
        }
    }
    
    void SomeQuestion()
    {
        bool loop = true;
    
        while(loop)
        {
            Console.WriteLine("Question secon level");
            Console.WriteLine("1. Ans3");
            Console.WriteLine("2. Ans4");
            Console.WriteLine("3. Exit");
            string resp = Console.ReadLine();
    
            switch(resp)
            {
                case "1":
                Console.WriteLine("Ans1 chosen");
                break;
                case "2":
                break;
                case "3":
                loop = false;
                break;
                default:
                Console.WriteLine("Invalid Input. Please try Again");
                break;        
            }
        }    
    }
    

    This is very simple, only one question but shows idea.

    • Instead goto loop with condition.
    • Instead if one switch which make code more readable.