[php] Error In If Statement

[glennda]

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

 

the error is

PHP Parse error: syntax error, unexpected '}'

 

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

 

Any ideas?

[Hightower]

Missing a ; before the last }

[powdarrmonkey]

if ($shirtquanity=='0')

 

1) You're comparing strings like numbers

2) what if $shirtquanity (sic) is negative?

Edited March 31, 2011 by powdarrmonkey

[glennda]

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!

[Hightower]

Is there another way i should be doing it?

 

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

 

if ($shirtquanity=='0')  

becomes

 

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

 

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

 

I prefer

 

 

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.....

Edited March 31, 2011 by Hightower

[glennda]

ah ok i will do that!

[powdarrmonkey]

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):

if ($shirtquanity=='0')

 

Numerical comparision (improvement):

if ($shirtquanity==0)

 

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

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.

[glennda]

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

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

 

So shouldn't that stop that?

[powdarrmonkey]

I've used

$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.

[powdarrmonkey]

$shirt = $shirtquanity." x Playing Shirt Size ".$shirtsize."
"

 

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

[glennda]

I'll take a look cheers

 

Toby