1
Console.Clear();
string choice;

Console.WriteLine("Welcome to Costa coffee\n");
Console.WriteLine("1:> Latte\n2:> Cappuccino\n3:> Espresso\n4:> Double espresso");
Console.WriteLine("\nPlease select a coffee by pressing 1-4");
choice = Console.ReadLine();


if (choice == "1")
{
    Console.WriteLine("\nYou have selected: latte");
}
if (choice == "2")
{
    Console.WriteLine("\nYou have selected: Cappuccino");
}
if (choice == "3")
{
    Console.WriteLine("\nYou have selected: Espresso");
}
if (choice == "4")
{
    Console.WriteLine("\nYou have selected: Double espresso");
}

else if (choice !="1" || choice !="2" || choice !="3" || choice !="4")
{
    Console.WriteLine("Incorrect value, please try again");
}

I'm trying to make the program so that if choice isn't equal to 1,2,3,4 then it will display "Incorrect value, please try again" however it works when I press anything random yet still displays this error message when I press 1,2,3 or 4. Any idea's?

7
  • It won't do it if you choose 4. It will do it if you choose 1, 2 or 3. Hint: what do you think the "else" is an alternative to? That will happen if which condition isn't met? (I'd suggest using a switch statement instead, mind you...) Commented Mar 6, 2017 at 22:51
  • just have the last else be else { your stuff } and chain else ifs. Commented Mar 6, 2017 at 22:51
  • 1
    choice != "1" && choice != "2" & ... Commented Mar 6, 2017 at 22:51
  • use a case default conditional block Commented Mar 6, 2017 at 22:51
  • Use switch... Commented Mar 6, 2017 at 22:52

7 Answers 7

2

Recommend switch in this case:

var myConsoleString = "";

switch (choice)
{
    case "1": myConsoleString = "\nYou have selected: latte"; break;
    case "2": myConsoleString = "\nYou have selected: Cappuccino"; break;
    case "3": myConsoleString = "\nYou have selected: Espresso"; break;
    case "4": myConsoleString = "\nYou have selected: Double espresso"; break;
    default:  myConsoleString = "\nIncorrect value, please try again"; break;
}

Console.WriteLine(myConsoleString);
Sign up to request clarification or add additional context in comments.

3 Comments

for ints you don't need the parentheses, but other than that you beat me to it. And also great pick on not using Console.WriteLine inside the switch
I believe original poster was going with strings, but I agree. So it was tailored for him (or her!) :)
int choice = Convert.ToInt32(Console.ReadLine()); would require an error check though.
1

Trivially adjusting your code (but there are better ways to write this):

Console.Clear();
string choice;

Console.WriteLine("Welcome to Costa coffee\n");
Console.WriteLine("1:> Latte\n2:> Cappuccino\n3:> Espresso\n4:> Double espresso");
Console.WriteLine("\nPlease select a coffee by pressing 1-4");
choice = Console.ReadLine();


if (choice == "1")
{
    Console.WriteLine("\nYou have selected: latte");
}
else if (choice == "2")
{
    Console.WriteLine("\nYou have selected: Cappuccino");
}
else if (choice == "3")
{
    Console.WriteLine("\nYou have selected: Espresso");
}
else if (choice == "4")
{
    Console.WriteLine("\nYou have selected: Double espresso");
}
else 
{
    Console.WriteLine("Incorrect value, please try again");
}

Comments

0

Your first three if statements are independent of the last two. You should add else before the if for choice == 2 and choice == 3. That way it all becomes one large if/else statement.

Comments

0

DRY version:

var coffee = null;

switch (choice)
{
    case "1": coffee = "Latte"; break;
    case "2": coffee = "Cappuccino"; break;
    case "3": coffee = "Espresso"; break;
    case "4": coffee = "Double espresso"; break;
}

Console.WriteLine(coffee == null ? "Incorrect value, please try again" : $"You have selected: {coffee}");

Comments

0

In your code there are two things wrong:

  1. The else if block is only executed when choice == "4" fails, you want it to run if all conditions fail.
  2. The condition itself doesn't check what you actually want to check, you want to display an error message if the choice is not 1 and is not 2 and is not 3 and is not 4. But you are using OR operator. So you could fix that by using AND but it won't solve the whole problem and it wouldn't be a good solution anyways.

What you need is a if - else if - else structure instead of consecutive if's.

if (choice == "1")
{
    Console.WriteLine("\nYou have selected: latte");
}
else if (choice == "2")
{
    Console.WriteLine("\nYou have selected: Cappuccino");
}
else if (choice == "3")
{
    Console.WriteLine("\nYou have selected: Espresso");
}
else if (choice == "4")
{
    Console.WriteLine("\nYou have selected: Double espresso");
}
else 
{
    Console.WriteLine("Incorrect value, please try again");
}

This is better than chain of if's because the other conditions will only be evaluated if the condition before it fails. For example there is no point to check if choice == "2" if the choice == "1" true. And the else block will be executed if all of the conditions fail which is what you wanted at the first place. Notice that there is no need to write a condition that is the reverse of all conditions, there is an else statement for this purpose.

Comments

0

You could use a dictionary to make the fact that "1" is "latte" and the other values more concrete and this would allow you to use the .Contains() method in place of your multiple if's or switch statement:

Console.Clear();
string choice;
Dictionary<string, string> choiceToCoffee = new Dictionary<string, string>()
{
    { "1", "Latte" },
    { "2", "Cappuccino" },
    { "3", "Espresso" },
    { "4", "Double Espresso" },
};
Console.WriteLine("Welcome to Costa coffee\n");
foreach (var kvp in choiceToCoffee)
{
    var thisChoice = kvp.Key;
    var thisCoffee = kvp.Value;
    Console.WriteLine(thisChoice + ":> " + thisCoffee);
}
Console.WriteLine("\nPlease select a coffee by pressing 1-4");
choice = Console.ReadLine();

if (choiceToCoffee.ContainsKey(choice))
{
    Console.WriteLine("\nYou have selected: " + choiceToCoffee[choice]);
}
else
{
    Console.WriteLine("Incorrect value, please try again");
}

Comments

0

I hate repeating myself, so I'd approach this with a view to cutting out the repetition.

By making a dictionary of valid choices, the logic to generate a message becomes much clearer and the adding of new options is less interwoven with the logic of deciding which choice was made:

var possibleChoices = new Dictionary<string,string>{
    {"1", "Latte"},
    {"2", "Cappuccino"},
    {"3", "Espresso"},
    {"4", "Double espresso"}
};

string value;

var message = possibleChoices.TryGetValue(choice, out value)
    ? string.Format("You have selected: {0}", value)
    : "Incorrect value, please try again";

//In your code, the fall-through case misses the leading \n .
//By avoiding repetition, consistency in formatting is achieved
//with a single line of code
Console.WriteLine("\n{0}", message); 

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.