0
\$\begingroup\$

I am calculating discounts based on a quantity after 6.

Is there a cleaner way to do this:

$leg_directory_discount = array(
    6   =>  .50,
    7   =>  .50,
    8   =>  .50,
    9   =>  .50,
    10  =>  .50,
    11  =>  1,
    12  =>  1,
    13  =>  1,
    14  =>  1,
    15  =>  1,
    16  =>  1,
    17  =>  1,
    18  =>  1,
    19  =>  1,
    20  =>  1,
    21  =>  1.5,
    22  =>  1.5,
    23  =>  1.5,
    24  =>  1.5,
    25  =>  1.5,
    26  =>  1.5,
    27  =>  1.5,
    28  =>  1.5,
    29  =>  1.5,
    30  =>  1.5,
    31  =>  1.5,
    32  =>  1.5,
    33  =>  1.5,
    34  =>  1.5,
    35  =>  1.5,
    36  =>  1.5,
    37  =>  1.5,
    38  =>  1.5,
);

edit: The max and min values of the discounts never change. Here is how I'm calculating the discount and arriving at the final price:

if($v[0]['leg_discount'] == '1' && $v[0]['qty'] >= 6){
    $discount = $qty * $leg_directory_discount[$qty];
    $price = $price - $discount;
}
\$\endgroup\$
1
  • 3
    \$\begingroup\$ What have tried? Can you add more details? \$\endgroup\$
    – Florent
    Oct 3, 2012 at 15:59

3 Answers 3

0
\$\begingroup\$

Some simple logic will save you from all that (error-prone) writing. For instance:

if ($qty >= 6)
{
    if ($qty >= 21)
        $leg_directory_discount = 1.5;
    elseif ($qty >=11)
        $leg_directory_discount = 1.0;
    else
        $leg_directory_discount = 0.5;
}
else
    $leg_directory_discount = 0.0;
\$\endgroup\$
1
  • \$\begingroup\$ Overly complicated, I'd say. How do you integrate a new discount for three products at 0.2 easily? \$\endgroup\$
    – Sven
    Oct 3, 2012 at 16:26
4
\$\begingroup\$

I like the idea of putting the discount steps into an array, but stating a discount for every possible quantity seems too much.

$leg_directory_discount = array(
    0 => 0,
    6   =>  .50,
    11  =>  1,
    21  =>  1.5,
);

This is the discount configuration - can later be stored in a config file or database.

This function gets the discount to be used:

function getDiscount($qty, $leg_directory_discount) {
  $return = 0;
  foreach ($leg_directory_discount as $amount => $discount) {
    if ($qty >= $amount) {
      $return = $discount;
    }
  }
  return $discount;
}
\$\endgroup\$
0
\$\begingroup\$

You could use a switch statement as well:

switch (true) {
    case ($price <=5):
         $discount = 0.0;
         break;
    case ($price <= 10):
         $discount = 0.5;
         break;
    case ($price <= 20):
         $discount = 1.0;
         break;
    default:
         $discount = 1.5;
         break;
}

Also your final price could be reduced to $price = $price * (100 - $discount) / 100;

\$\endgroup\$
2
  • \$\begingroup\$ Your switch statement looks bogus. Shouldn't it be at least switch (true) if you want to evaluate on every case line. You might get along because $price evaluates to true most of the time. I would try to avoid switch nonetheless - makes code overcomplicated in the long run. \$\endgroup\$
    – Sven
    Oct 3, 2012 at 16:18
  • \$\begingroup\$ @Sven: your probably right, edited per your comment. \$\endgroup\$
    – Jeffory J. Beckers
    Oct 3, 2012 at 16:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.