Question

I have my website built in ASP.NET 2.0 and C#. I have textbox called tbCode where a user enters a code. I am trying to check that entered value against multiple values in the code behind on a button click.

This is my mark up so far.

 protected void btUpdate_Click(object sender, EventArgs e)
{


    if ((this.tbcode.Text.Trim().ToUpper() != "AB12") ||(this.tbcode.Text.Trim().ToUpper() != "DE14") || (this.tbcode.Text.Trim().ToUpper() != "XW16"))
    {
        lbmessage.Text = "Invalid Promo code. Please enter again";
    }
    else if ((this.tbcode.Text.Trim().ToUpper() == "AB12") || (this.tbcode.Text.Trim().ToUpper() == "DE14") || (this.tbcode.Text.Trim().ToUpper() == "XW16"))
    {
        Order.Shipping.Cost = 0;
        this.lShipping.Text = Order.Shipping.Cost.ToString("c");
        this.lSubtotal.Text = Order.Subtotal.ToString("c");
        this.lTotal.Text = Order.TotalCost.ToString("c");
        Order.PomoCode = this.tbcode.Text.Trim().ToUpper();
        lbmessage.Text = "Promo Code Applied.";
    }
    else
    {
        this.lShipping.Text = Order.Shipping.Cost.ToString("c");
        this.lSubtotal.Text = Order.Subtotal.ToString("c");
        this.lTotal.Text = Order.TotalCost.ToString("c");

    }

}

when i hit the button its always saying invalid code. not sure where am i making the mistake. It works perfectly if I am checking against just one value rather than the 3.

Thanks and appreciate it

Was it helpful?

Solution

Here is likely what you wanted to do:

protected void btUpdate_Click(object sender, EventArgs e)
{
    string tbcodeValue = this.tbcode.Text.Trim().ToUpper();

    string[] validCodes = new string[] { "AB12", "DE14", "XW16" };
    if (!validCodes.Contains(tbcodeValue))
    {
        lbmessage.Text = "Invalid Promo code. Please enter again";
    }
    else 
    {
        Order.Shipping.Cost = 0;
        this.lShipping.Text = Order.Shipping.Cost.ToString("c");
        this.lSubtotal.Text = Order.Subtotal.ToString("c");
        this.lTotal.Text = Order.TotalCost.ToString("c");
        Order.PomoCode = tbcodeValue;
        lbmessage.Text = "Promo Code Applied.";
    }
}

First off, you were calling this.tbcode.Text.Trim().ToUpper() all over the place. That really clutters up your code and makes it hard to read. Assigning that to a variable not only makes the code cleaner, but avoids performing all of those string manipulation functions over and over.

Next, it appears that your intent is to say, "if the textbox value isn't any of these values, run some code saying it's invalid. The easiest way to do that is to put all of the valid values into a container of some sort and see if it contains the value you're interested in. Your next block of code is basically for, "if it is one of the valid values". So if it does contain the string then it is valid. As for your else, I couldn't figure out what the intent of it was. Either the string is invalid, or it's valid. I don't see any third case there, so I just removed it.

OTHER TIPS

You need to change your ||'s to &&'s in the first if statement. You are always going to fall into that block otherwise.

Try this:

 if ((this.tbcode.Text.Trim().ToUpper() != "AB12") && (this.tbcode.Text.Trim().ToUpper() != "DE14") && (this.tbcode.Text.Trim().ToUpper() != "XW16"))
    {
        lbmessage.Text = "Invalid Promo code. Please enter again";
    }
    else if ((this.tbcode.Text.Trim().ToUpper() == "AB12") || (this.tbcode.Text.Trim().ToUpper() == "DE14") || (this.tbcode.Text.Trim().ToUpper() == "XW16"))
    {
        Order.Shipping.Cost = 0;
        this.lShipping.Text = Order.Shipping.Cost.ToString("c");
        this.lSubtotal.Text = Order.Subtotal.ToString("c");
        this.lTotal.Text = Order.TotalCost.ToString("c");
        Order.PomoCode = this.tbcode.Text.Trim().ToUpper();
        lbmessage.Text = "Promo Code Applied.";
    }
    else
    {
        this.lShipping.Text = Order.Shipping.Cost.ToString("c");
        this.lSubtotal.Text = Order.Subtotal.ToString("c");
        this.lTotal.Text = Order.TotalCost.ToString("c");

    }

You could also employ a switch;case; block.

String testtext = this.tbcode.Text.Trim().ToUpper();

switch(testtext)
{
    case "AB12":
        // Do stuff for this case
        break;
    case "Text2":
        // Do stuff for this case
        break;
    default:
        // anything that fails all above tests goes here
        break;
}

Just be sure to either break, return, or continue after each case or you will get a compile error. Also default needs to be last in line.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top