Error In If Statement

I've written this if statement but cannot see what is wrong

the error is
PHP Parse error: syntax error, unexpected '}'

PHP Code:

---

if ($shirtquanity=='0') {$shirt = "";} else {$shirt = $shirtquanity." x Playing Shirt Size ".$shirtsize."<br />"} 


---

Any ideas?

Missing a ; before the last }

Quote:

---

Originally Posted by glennda

PHP Code:

---

if ($shirtquanity=='0') 


---

---

1) You're comparing strings like numbers
2) what if $shirtquanity (sic) is negative?

The value is a number an always will be as it gets it from a dropdown menu on previous page (order quantity!)

Is there another way i should be doing it?

And @hightower - thanks i've been staring at code for to long and nobody here knows php and i'm no expert!

Quote:

---

Originally Posted by glennda

Is there another way i should be doing it?

---

Yeah, you should perhaps remove the single quotes from the number so

PHP Code:

---

if ($shirtquanity=='0') 


---

becomes

PHP Code:

---

if ($shirtquanity==0) 


---

That way you are telling PHP to expect a number instead of a string and thus it can better handle it.

Also, I don't like single line if statements like

PHP Code:

---

if (x == y) { //do this } else { //do this } 


---

I prefer

PHP Code:

---

if (x == y)
{
    //do this
}
else
{
    //do this
} 


---

Nothing wrong with your way, just personal preference. I find in my second example it's easier to read and find errors in the code, plus if you get paid per line..... ;)

ah ok i will do that!

Quote:

---

Originally Posted by glennda

The value is a number an always will be as it gets it from a dropdown menu on previous page (order quantity!)

---

I have a funny sense of deja vu...

String comparison (wrong):

PHP Code:

---

if ($shirtquanity=='0') 


---

Numerical comparision (improvement):

PHP Code:

---

if ($shirtquanity==0) 


---

Numerical comparison, without trusting the user input (good):

PHP Code:

---

define('MAX_ORDER_QUANTITY', 50);
if ( is_numeric($shirtquanity) && $shirtquanity > 0 && $shirtquanity < MAX_ORDER_QUANTITY)
{
   // fulfill order
} else {
   // tell the user
} 


---

Never, ever ever ever trust user input. Just because you've supplied a dropdown in the user agent, that doesn't me injecting values you weren't expecting (like a negative number, or worse a SQL injection attack). Your form is only a hint to the user agent.

Quote:

---

Originally Posted by powdarrmonkey

Never, ever ever ever trust user input. Just because you've supplied a dropdown in the user agent, that doesn't me injecting values you weren't expecting (like a negative number, or worse a SQL injection attack). Your form is only a hint to the user agent.

---

I've used

PHP Code:

---

$shirtquantity =  mysql_real_escape_string($_POST['quantityshirt'] 


---

So shouldn't that stop that?

Quote:

---

Originally Posted by glennda

I've used

PHP Code:

---

$shirtquantity =  mysql_real_escape_string($_POST['quantityshirt'] 


---

So shouldn't that stop that?

---

If you were handling a string, yes (the clue is in the name). If you're handling a number, which you are, the value checking I already posted is sufficient. Otherwise, mysql_real_escape_string() casts its return value to a string and you have the same comparison problem.

Quote:

---

Originally Posted by glennda

PHP Code:

---

$shirt = $shirtquanity." x Playing Shirt Size ".$shirtsize."<br />" 


---

---

Hum... re-reading this bit, mysql_real_escape_string() also doesn't protect you from the cross-site scripting attack that this line contains.

I'll take a look cheers

Toby