7

Im trying to code a simple text adventure. when I started to code the directions a user can head in I realized the user could put in "north east", "south west" etc so I thought I should make cases for them.

my problem is case5 "north east" does not run when I input "north east" into the command line.

class Branches

    {
        static void main(string[] args)
        {
            Branch2();
        }

        public static void Branch2()
        {
            Console.WriteLine("");
            Console.WriteLine("(North ,East ,South ,West)");
            string input = Console.ReadLine();
            bool case1 = input.Contains("north");
            bool case2 = input.Contains("east");
            bool case3 = input.Contains("south");
            bool case4 = input.Contains("west");
            bool case5 = input.Contains("north") && input.Contains("east");

            //Console.ReadLine(); 
            int CaseId;
            if (case1)
                CaseId = 1;
            else if (case2)
                CaseId = 2;
            else if (case3)
                CaseId = 3;
            else if (case4)
                CaseId = 4;
            else if (case5)
                CaseId = 5;

            else
                CaseId = 0;

            switch (CaseId)
            {
                case 1:
                    Console.WriteLine("");
                    Console.WriteLine("you head north");
                    break;

                case 2:
                    Console.WriteLine("");
                    Console.WriteLine("you head east");
                    break;

                case 3:
                    Console.WriteLine("");
                    Console.WriteLine("you head south");
                    break;

                case 4:
                    Console.WriteLine("");
                    Console.WriteLine("you head west");
                    break;

                case 5:
                    Console.WriteLine("");
                    Console.WriteLine("you head north east");
                    break;

                default:
                    Console.WriteLine("enter a valid direction");
                    break;


            }
        }
    }

}
Charlie
  • 83
  • 1
  • 2
  • 6

4 Answers4

13

Because input contains north so first if is executed and not searching for else if, try to inverse if's and put case5 as first if :)

            if (case5)
                CaseId = 5;
            else if (case2)
                CaseId = 2;
            else if (case3)
                CaseId = 3;
            else if (case4)
                CaseId = 4;
            else if (case1)
                CaseId = 1;
Kamil Budziewski
  • 22,699
  • 14
  • 85
  • 105
  • I wouldn't inverse the logic, rather simply move the last statement to be the first - e.g. "if (case5) ...; else if (case1) ...; ...". Anywho thumbs up. – NPSF3000 Jul 08 '13 at 08:35
4

While this has been answered, I'd make my one observation:

Your booleans are unnecessary, and help hide the problem.

For example this:

        string input = Console.ReadLine();
        bool case1 = input.Contains("north");
        bool case2 = input.Contains("east");
        bool case3 = input.Contains("south");
        bool case4 = input.Contains("west");
        bool case5 = input.Contains("north") && input.Contains("east");

        int CaseId;
        if (case1)
            CaseId = 1;
        else if (case2)
            CaseId = 2;
        else if (case3)
            CaseId = 3;
        else if (case4)
            CaseId = 4;
        else if (case5)
            CaseId = 5;

is the same as this:

        string input = Console.ReadLine();
        int CaseId;

        if (input.Contains("north")) CaseId = 1;
        else if (input.Contains("east")) CaseId = 2;
        else if (input.Contains("south")) CaseId = 3;
        else if (input.Contains("west")) CaseId = 4;
        else if (input.Contains("north") 
                && input.Contains("east")) CaseId = 5;

Which should make it fairly apparent that the statement 'input.Contains("north") && input.Contains("east")' will never be run - as it requires 'input.Contains("north')' to be false (thanks to the 'else'). Debugging and stepping through do help a lot here.

So yeah, as a programmer true to be succinct with your code - never write more than you need to. In your code, you have a bunch of booleans, a bunch of if's and a switch statement - all effectively doing the same job.

I'd personally write it like so:

        string input = Console.ReadLine();

        if (input.Contains("north") && input.Contains("east")) 
            Console.WriteLine("\nyou head north east");
        else if (input.Contains("north")) 
            Console.WriteLine("\nyou head north");
        else if (input.Contains("east")) 
            Console.WriteLine("\nyou head east");
        else if (input.Contains("south")) 
            Console.WriteLine("\nyou head south");
        else if (input.Contains("west"))                    
            Console.WriteLine("\nyou head west");
        else 
            Console.WriteLine("enter a valid direction");

It's just the same in readability and understandability, but involves a third the lines and therefor a third of the chance of an error.

NPSF3000
  • 2,421
  • 15
  • 20
  • Thanks, im new with coding but this makes it clear. I really am surprised how much unnecessary code is in my program, what I have made was kind of hacked together but its stuff like this that helps me advance. – Charlie Jul 08 '13 at 09:09
  • Glad to be of assistance :) It doesn't apply in this particular situation, but make sure you are well versed in both arrays and loops before going too far - they'll massively help you down the track. – NPSF3000 Jul 09 '13 at 02:31
2

"north east"

bool case1 = input.Contains("north"); //true
bool case5 = input.Contains("north") && input.Contains("east"); //true

if (case1)
      CaseId = 1;
else //Done moving on
awiebe
  • 3,758
  • 4
  • 22
  • 33
1

It can't work becuase you are asking "contains North" as the first question? the answer is yes. The you go strait to step 1. you don't get to step 5.

In order to get to step 5 you need to ask it first.

Anyway it looks like there are more issues in the code. I am not sure what you are trying to achieve, but it looks that you complicate things for no reason.

I would do something similar to the following:

List<string> eastWest = new List<string> {"east", "west"};
            var up = new List<string> {"north", "south"};
            var direction = Console.ReadLine();
            var upDirection = up.Find(direction.Contains);
            var sideDirection = eastWest.Find(direction.Contains);
            var result = upDirection != null ? upDirection + " " + sideDirection:sideDirection;
            Console.WriteLine("You are headed" + result);
user844541
  • 2,868
  • 5
  • 32
  • 60