Comparing string - woes in PHP

Question

I want to allow the user to enter a coupon number in order to get a discount. After the coupon number is entered and submitted, the page reloads withe a tick showing that they have entered a correct amount.

The way I'm trying to do this is displaying the tick if the coupon amount is not £0.00. But the string comparison doesn't seem to work, since it always thinks that it is not £0.00. The code is as follows. The function coupon_amount() returns the coupon amount. coupon_amount() returns "£0.00" (including the pound sign)

<?php $coup_amount = coupon_amount(); ?>
<?php $zero_amount = "£0.00"; ?>

<?php if(strcmp($coup_amount, $zero_amount)== 0) { ?>

 <?php echo 'Enter coupon code if applicable:' ?>
 <input type='text' class='couponinput' name='coupon_num' id='coupon_num' value='coupons_name' />
 <input type='submit' class='update-button' value='submitcoupon' />

<?php } else {  ?>

 <?php echo 'Thanks.' ?><input type='text' disabled='disabled' class='couponinput' name='coupon_num' id='coupon_num' value='coupons_name' />
 <div class='tick'></div>

<?php }  ?>

Am I doing something wrong with the comparison?

I followed Oscar's suggestion below, and here is the output. Seems to be an encoding problem. And the pound sign is not appearing properly for the zero_amount.

coup_amount: (£0.00)  zero_amount: (�0.00) 
coup_len:10 zero_len:5
strcmp: -1
coup_ascii: 38 zero_ascii:163

Answer 1

the strcmp call seems ok, my bet is on the coupon_amount function.

Answer 2

You should really be storing/working with the discount values as numbers, this would make the comparison much easier.

Answer 3

<?php if(strcmp($coup_amount, $zero_amount)== 0) { ?>

Seems very unreadable compared to:

<?php if(coupon_amount() == 0) { ?>

If coupon_amount() returned the actual value, not the formatted string representation.

Are you able to change the coupon_amount() function to get rid of the pound symbol? the php function money_format is good for adding whatever the users currency symbol is to a string for displaying on the page (or which ever symbol you set the locale to)

In the future you will find yourself having to remove the pound symbol first before you do any arithmetic on the return value from coupon_amount()

Answer 4

There are a lot of things that seem insignificant to user, but may break string comparison

• coupon_amount() may insert some spaces somewhere in returned value
• coupon_amount() may return variable number of zeros
• coupon_amount() may use comma instead of dot (depending on locale)
• coupon_amount() may encode pound sign using some HTML entity

Said that, it is much better to compare numeric values, and then format number as currency.

Answer 5

Have you tried printing out all the values?

<?php $coup_amount = coupon_amount(); ?>
<?php $zero_amount = "£0.00"; ?>

//print'em out
<pre>
<?php 
  echo "coup_amount: ($coup_amount)  zero_amount: ($zero_amount) \n";
  echo "coup_len:".strlen($coup_amount)." zero_len:".strlen($zero_amount)."\n";
  echo "strcmp: ".strcmp($coup_amount, $zero_amount)."\n";
  echo "coup_ascii: ".ord($coup_amount[0])." zero_ascii:".ord($zero_amount[0]);
?>
</pre>

Amendment
So yes, now that we can see the output from this, it seems like the coup-string is an UTF16 (10 bytes long) and the other is something else (5 bytes long).

(Preaching follows.) When dealing with money you should really take extra care to make sure numbers are handled correctly. We've just seen that strings are subject to encoding, and like others have pointed out, floats are subject to fractional errors. Your best bet is probably to try and express it in 1/100s using an integer, and to express the currency in a separate variable. (Preaching over.)

But I'm guessing the coupon_amount-function is used everywhere and you can't change it. Then you might wanna look into converting the two strings so that the are in the same format. Take a look at iconv.

Answer 6

The absurd lengths I had to go to to get this working :) .. I changed the first if statment to:

if((ord($coup_amount[0])==38) && (ord($coup_amount[1])==35) 
&& (ord($coup_amount[2])==49) && (ord($coup_amount[3])==54) 
&& (ord($coup_amount[4])==51) && (ord($coup_amount[5])==59) 
&& (ord($coup_amount[6])==48) && (ord($coup_amount[7])==46) 
&& (ord($coup_amount[8])==48) && (ord($coup_amount[9])==48) 
&& (ord($coup_amount[10])==0))