C# nested if/else in a for loop

Question

I have a nested if/else statement in a for loop to determine whether or a a value is valid with an array value. It returns all values just fine, however if the IF is correct, it still does the else an additional three times. I thought once it was equal one time, it would stop, but I suppose I am missing something here.

string sectionChoice;
int ticketQuantity;
double ticketPrice, totalCost;
string[] section = { "orchestra", "mezzanine", "balcony", "general" };
double[] price = { 125.25, 62.00, 35.75, 55.50 };
bool isValidSection = false;

sectionChoice = GetSection();
ticketQuantity = GetQuantity();

for (int x = 0; x < section.Length; ++x)
{
    if (sectionChoice == section[x])
    {
        isValidSection = true;
        ticketPrice = price[x];

        totalCost = CalcTicketCost(ticketPrice, ticketQuantity);
        Console.Write("\n\nTotal cost for the tickets are: {0:c2}", totalCost);
    }
    else
        Console.Write("\n\nInvalid entry, {0} does not exist", sectionChoice);
}

When it is valid, it returns something like this:

Your price is 32.50. Invalid entry, x does not exist Invalid entry, x does not exist Invalid entry, x does not exist

You should be using decimal for currency values (read more). — David Sherret
– David Sherret, Commented Jun 18, 2015 at 18:22

Servy · Accepted Answer · 2015-06-18 18:26:20Z

4

What you're really trying to do is determine if section contains a particular value, and do something if it does. Just check that directly:

if (section.Contains(sectionChoice))

You also shouldn't be using parallel arrays. Rather than having two arrays, sections an prices, in which the object at the index of each both "combine" to equal a value, it appears that what you're actually modeling is a means to lookup the price of a particular section. This is best modeled with a Dictionary that can easily look up the value for a particular key. Here your key is the section, and the value is its price.

Dictionary<string, decimal> ticketsPrices = new Dictionary<string, decimal>()
{
    {"orchestra", 125.25m},
    //...
};
bool isValidSection = false;

string sectionChoice = GetSection();
int ticketQuantity = GetQuantity();

if (ticketsPrices.ContainsKey(sectionChoice))
{
    isValidSection = true;
    decimal ticketPrice = ticketsPrices[sectionChoice];

    decimal totalCost = CalcTicketCost(ticketPrice, ticketQuantity);
    Console.Write("\n\nTotal cost for the tickets are: {0:c2}", totalCost);
}
else
    Console.Write("\n\nInvalid entry, {0} does not exsist", sectionChoice);

edited Jun 18, 2015 at 18:26

answered Jun 18, 2015 at 18:14

Servy

204k27 gold badges347 silver badges470 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

adv12 Over a year ago

This answer surpasses the others by identifying the intent and solving the problem a better way.

David Sherret Over a year ago

What happens with x? You still need the index to get the price, but this is the right idea (or a dictionary could be used with Dictionary<string, decimal>)

adv12 Over a year ago

@DavidSherret, good point. Didn't see that initially. Could be solved by using Array.IndexOf and would still be cleaner than the other answers.

Servy Over a year ago

@DavidSherret yeah, the design was even more flawed than I noticed at first glance. This is why you avoid parallel arrays; it creates a giant mess.

Servy Over a year ago

@adv12 The solution is to get rid of the parallel arrays, rather than trying to work around them. They'll only continue to act up down the road.

DotNetRussell · Accepted Answer · 2015-06-18 18:14:38Z

1

The keyword you are looking for is break;

break will stop the execution of the loop it is inside. If you are inside nested loops it will only work on the innermost.

The opposite of this is continue. Continue stops that iteration and moves onto the next.

Here is an MSDN article explaining it more in depth

for (int x = 0; x < section.Length; ++x)
{
   if (sectionChoice == section[x])
   {
      isValidSection = true;
      ticketPrice = price[x];

      totalCost = CalcTicketCost(ticketPrice, ticketQuantity);
      Console.Write("\n\nTotal cost for the tickets are: {0:c2}", totalCost);
      break;
   }
   else
     Console.Write("\n\nInvalid entry, {0} does not exsist", sectionChoice);
 }

edited Jun 18, 2015 at 18:14

answered Jun 18, 2015 at 18:11

DotNetRussell

9,87511 gold badges61 silver badges122 bronze badges

4 Comments

DotNetRussell Over a year ago

@JeffreyDilley please don't forget to select this as an answer by clicking the checkmark if this worked for you.

Podo Over a year ago

Absolutely. It says I have to wait a few minutes, but I most certainly will.

Olivier Jacot-Descombes Over a year ago

You still might get the invalid entry message several times, if the selected section is not the first!

DotNetRussell Over a year ago

@OlivierJacot-Descombes the question was how to break a loop. Not to fix the logic behind it. Let him learn it through trial and error.

Olivier Jacot-Descombes · Accepted Answer · 2015-06-18 18:43:33Z

Get the index of the chosen item like this:

int i = section.IndexOf(sectionChoice);
if (i >= 0) {
    ticketPrice = price[i];
} else {
    // Not found!
}

However your code would become more flexible with an item class

public class Section
{
    public string Name { get; set; }
    public decimal Price { get; set; }
}

With this class you can create a list of sections like this

List<Item> sections = new List<item> {
    new Section { Name = "orchestra", Price = 125.25 },
    new Section { Name = "mezzanine", Price = 62.00 },
    ...
};

Now you can find a section like this:

Section section = sections.FirstOrDefault(s => s.Name == sectionChoice);
if (section != null ) ...

Like this it is easier to add new properties to the sections. E.g. you could add a number of remaining seats to it, without having to create a new array.

Mike · Accepted Answer · 2015-06-18 18:23:11Z

0

You don't want to report that it's an invalid choice for each one that it doesn't match:

for (int x = 0; x < section.Length; ++x)
{
    if (sectionChoice == section[x])
    {
        isValidSection = true;
        ticketPrice = price[x];

        totalCost = CalcTicketCost(ticketPrice, ticketQuantity);
        Console.Write("\n\nTotal cost for the tickets are: {0:c2}", totalCost);
        break;
     }
}

if (!isValidSection)
    Console.Write("\n\nInvalid entry, {0} does not exist", sectionChoice);

answered Jun 18, 2015 at 18:23

Mike

4352 silver badges7 bronze badges

2 Comments

Mike Over a year ago

Could I request a reason for the down votes? This is only my third post. I'm not sure where it's lacking.

Olivier Jacot-Descombes Over a year ago

In fact, the invalid entry message must be placed after the loop, otherwise it still might be written to the console several times!

Olivier Jacot-Descombes · Accepted Answer · 2015-06-18 20:08:20Z

0

If you want iteration to stop when the if condition is met, you need to break out of the loop with a break statement:

for (int x = 0; x < section.Length; ++x)
{

    if (sectionChoice == section[x])
    {
        isValidSection = true;
        ticketPrice = price[x];

        totalCost = CalcTicketCost(ticketPrice, ticketQuantity);
        Console.Write("\n\nTotal cost for the tickets are: {0:c2}", totalCost);
        break;  // THIS IS THE IMPORTANT CHANGE
    }

}
if (!isValidSection) // TEST MOVED OUTSIDE OF LOOP
{
    Console.Write("\n\nInvalid entry, {0} does not exsist", sectionChoice);
}

edited Jun 18, 2015 at 20:08

Olivier Jacot-Descombes

114k14 gold badges149 silver badges202 bronze badges

answered Jun 18, 2015 at 18:12

adv12

8,5512 gold badges28 silver badges52 bronze badges

1 Comment

Olivier Jacot-Descombes Over a year ago

You still might get the invalid entry message several times, if the selected section is not the first!

Collectives™ on Stack Overflow

C# nested if/else in a for loop

5 Answers 5

5 Comments

4 Comments

Comments

2 Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

5 Comments

4 Comments

Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related