How to end an IF?

Question

I'm stuck on something... In my program there is credits... I have products which are all equal to a credit. So after the buyer has used up all their credits they shouldn't be able to purchase anymore.. but it keeps going and going into the minuses - and I can't figure out how to stop it.

This is the code

private void CmdProgBar_Click(object sender, EventArgs e)
{
    int Credits = Convert.ToInt32(lblcredits.Text);

    if (Credits <= 0)
    {
        MessageBox.Show("You do not have enough credits");
    }
    else

    if (comboBox1.SelectedIndex == 0)
    {
        MessageBox.Show("You've selected Pair");
        Credits -= 1;
        lblcredits.Text = Convert.ToString(Credits);
    }

    if (comboBox1.SelectedIndex == 1)
    {
        MessageBox.Show("You've selected Banana");
        Credits -= 2;
        lblcredits.Text = Convert.ToString(Credits);
    }

    if (comboBox1.SelectedIndex == 2)
    {
        MessageBox.Show("You've selected Apple");
        Credits -= 3;
        lblcredits.Text = Convert.ToString(Credits);
    }
}

Answer 1

It seems like you will need to correct some of the code. Below is my solution for you. I added comments for you to understand my answer for you. Keep in mind, if they have 1 credit, then they can go to negative if they choose an apple or a banana. If you need to have enough credits to purchase an apple or a banana, then you must verify they have enough credits for that (I included the code after the answer).

private void CmdProgBar_Click(object sender, EventArgs e) //After the buyer has used up all their credits they shouldn't be able to purchase anymore
{
    int Credits = Convert.ToInt32(lblcredits.Text);

    if (Credits <= 0) //IF credits are less than or equal to zero, the show message
    {
        MessageBox.Show("You do not have enough credits");
    }
    else //Meaning more than zero credits, then see what they have selected
    {
        if (comboBox1.SelectedIndex == 0)
        {
            MessageBox.Show("You've selected Pear"); //Correct term from PAIR to PEAR
            Credits -= 1;  //If credits are equal to 1, remaining credits equal 0
        }
        else if (comboBox1.SelectedIndex == 1)
        {
            MessageBox.Show("You've selected Banana");
            Credits -= 2; //If credits are equal to 1, remaining credits equal -1
        }
        else if (comboBox1.SelectedIndex == 2)
        {
            MessageBox.Show("You've selected Apple");
            Credits -= 3; //If credits are equal to 1, remaining credits equal -2
        }
        lblcredits.Text = Convert.ToString(Credits); //Move down because this will always be used after a selection occurs

    }//End else
}

So if you need to check if they have enough credits for what they have selected, do the following:

private void CmdProgBar_Click(object sender, EventArgs e) //After the buyer has used up all their credits they shouldn't be able to purchase anymore
{
    int Credits = Convert.ToInt32(lblcredits.Text);
    int cost = comboBox1.SelectedIndex + 1; //Assuming cost is equal to selected index plus 1

    if( Credits >= cost)
    {
        if (comboBox1.SelectedIndex == 0)
        {
            MessageBox.Show("You've selected Pear"); //Correct term from PAIR to PEAR
        }
        else if (comboBox1.SelectedIndex == 1)
        {
            MessageBox.Show("You've selected Banana");
        }
        else if (comboBox1.SelectedIndex == 2)
        {
            MessageBox.Show("You've selected Apple");
        }
        Credits -= cost;
        lblcredits.Text = Convert.ToString(Credits); //Move down because this will always be used after a selection occurs
    } //if
    else
    {
        MessageBox.Show("You do not have enough credits");    
    }//End else
}

Let me know what you think :) Hope that helps!

Answer 2

You can use the return statement.

private void DoSomething()
{
    if (someCondition)
    {
        // Some code that does stuff here.

        // Exit the method.
        return;
    }

    // If the above condition is true then this code is not hit.
    // ....
    this.DoSomeOtherStuff();
}

Or if the method has a return value.

private SomeObject DoSomething()
{
    if (someCondition)
    {
        // Some code that does stuff here.

        // Exit the method.
        return null;
    }

    // If the above condition is true then this code is not hit.
    // ....
    this.DoSomeOtherStuff();
}

I think you would be better off with a switch statement though.

Switch statements and loops use the break statement.

    int Credits = int.Parse(lblcredits.Text);

    if (Credits <= 0)
    {
        MessageBox.Show("You do not have enough credits");
        return;
    }

    var message = string.Empty;
    var value = comboBox1.SelectedIndex;

    switch (value)
    {
        case 0:
            message = "You've selected Pair";
            break;

        case 1:
            message = "You've selected Banana";
            break;

        case 2:
            message = "You've selected Apple";
            break;

        default:
            // Do default behaviour here, or throw an exception.
            throw new InvalidOperationException("Selected Index Invalid");
    }

    MessageBox.Show(message);

    Credits -= (value + 1);
    lblcredits.Text = Credits.ToString();

Hope you don't mind but I've refactored a few things too. Can't help myself :)

Answer 3

You have several issues here.

1. You only check to see if the user has a positive number of credits. You then subtract any number from that based on their selection. Even if I, as a user, only had a single credit your code lets me buy an Apple.

2. You need more braces to wrap your statement if you're using this code structure.

Personally, I suggest doing things a little differently. I would introduce at least a couple of other structures to make everything easier:

var items = new Dictionary<int, Tuple<string, int>>();
items.Add(0, new Tuple("Pair", 1));
items.Add(1, new Tuple("Banana", 2));
items.Add(2, new Tuple("Apple", 3));

var selectedItem = items[comboBox1.SelectedIndex];
if(Credits >= selectedItem.Item2)
{
    MessageBox.Show(string.Format("You selected {0}", selectedItem.Item1));
    Credits -= selectedItem.Item2;
    lblCredits.Text = Credits.ToString();
}

You could easily create a named class to replace the Tuple, which would make your code easier to read. I simply used it to demonstrate how the data structure could reduce the complexity of your original code.

You can now add as many items as you need to the Dictionary without having to modify the if statement.

Answer 4

In C#, if you do not put { } around the statements following an if, else if, or else statement, the condition only affects the next statement.

You need to put braces around your else:

int Credits = Convert.ToInt32(lblcredits.Text);

if (Credits <= 0)
{
    MessageBox.Show("You do not have enough credits");
}
else
{
   ...
}

Answer 5

Your problem is that the else is only applying to the first if that follows it. Try wrapping them all in curly braces:

if (Credits <= 0)
{
    MessageBox.Show("You do not have enough credits");
}
else
{
    // Your 3 other if statements go here
}