Simplify code?

[gandalf458]

I have only just touched the surface of PHP's functionality. There are just so many ways to do things.

I wondered if there was a simpler/more efficient way to express the following:

$fees = 0;
if     ( $price > 500000 ) $fees = 750;
elseif ( $price > 350000 ) $fees = 550;
elseif ( $price > 250000 ) $fees = 500;
elseif ( $price > 200000 ) $fees = 460;
elseif ( $price > 150000 ) $fees = 420;
elseif ( $price > 100000 ) $fees = 385;
elseif ( $price > 0 )      $fees = 350; 

Thx G

[Force Flow]

You could use a switch-case statement instead of a chain of if-statements.

[gandalf458]

Thanks squire. I had thought of the switch-case statement - I've never used it yet! I'm not sure it would be simpler...

[cpradio]

I'm not sure a switch-case would work, as you are needing to do a comparison to determine which fee amount to apply. switch statements accept a value and you can then create case statements for the values you expect and perform different operations.

Another technique you could utilize is using an array to store your price threshold and the fee associated when that threshold is met, then you can loop through the array to determine which fee to apply. However, that is really just hiding the underlying process you are trying to do, and the if/else statements are much clearer to their intent from a readability stand point.

So I guess, my recommendation is to leave it as is, or possibly place it in a table that you can run a query against (again, really is somewhat hiding the process, but the query will be just as readable as to its intent).

[gandalf458]

Thanks Cpradio. G

[aaarrrggh]

A good way to achieve this in object oriented code would be to use a design pattern called the strategy design.

[DaveMaxwell]

How often are these fees going to change? You might want to consider putting them into a database table. If you would setup the table with a minimum and maximum dollar amount, you could do it in one query ($result = mysql_query("SELECT fee FROM AddtionalFee WHERE " . $price . " IS BETWEEN LowPrice and HighPrice");).

Then if you want to change the fees or add a range, you just have to make a database change.

[gandalf458]

Thanks guys.

[ScallioXTX]

Just for the heck of it:

$fees = 350;
foreach(array(10000 => 385, 15000 => 420, 20000 => 460, 25000 => 500, 35000 => 550, 50000 => 750) as $p => $fee)
{
    if ($p >= $price)
    {
       $fees = $f;
       continue;
    }
    break;
} 

I started $fees on 350 assuming that price will always be more than 0. If it's not you should add it to the array (at the front).

[aaarrrggh]

I don't think this is cleaner code though. It's shorter, but I actually find it more difficult to read...

Just for the heck of it:

$fees = 350;
foreach(array(10000 => 385, 15000 => 420, 20000 => 460, 25000 => 500, 35000 => 550, 50000 => 750) as $p => $fee)
{
    if ($p >= $price)
    {
       $fees = $f;
       continue;
    }
    break;
} 

I started $fees on 350 assuming that price will always be more than 0. If it's not you should add it to the array (at the front).

[ScallioXTX]

I don't think this is cleaner code though. It's shorter, but I actually find it more difficult to read...

I agree. The best (cleanest) way to solve this is to do away with the current fee structure and get a formula instead, but I don't think that's an option.

[gandalf458]

Thanks Aaarrrggh. I think that might have been more what I had in mind but, yes, whether it's any more readable... Anyway, thanks for an alternative way to code it...