Discount calculation



  • Just bumped against this code on my current project:

     
    if($_REQUEST['discount_price'][$key] != 0) //avoid division by 0
    {   
        $value = floatval($_REQUEST['list_price'][$key]);
        $discount = - 100 + ($value * 100) / $value;       
        $discount = round($discount, 2);
    }
    else
        $discount = 100;
    

    With discounts like these, I think I'll stick to the list price, thank you.



  • Wow.  That's wrong in so many different ways.  It's beautiful, if you love looking at train wrecks. 



  • eh... your discount is either 0 or 100.

    why doesnt it just say $discount = $_REQUEST['discount_price'][$key] == 0?100:0;

    edit: not to mention, the user can get themselves the discount by setting the cookie themselves. Not that they could do too much harm since they couldnt get anything but a 0 or 100.



  • @darkmattar said:

    why doesnt it just say $discount = $_REQUEST['discount_price'][$key] == 0?100:0;

    Don't forget that you could still have a division by 0 if $_REQUEST['list_price'][$key] == 0    :)

    Anyway, I've corrected the code to, you know, something that actually makes sense.

     

    Edit: also, it's not a cookie, but the value of an input field, named something like "discount_price[2]". And it is set by the user anyway (we're talking back-office here).



  • @Zecc said:

    Just bumped against this code on my current project:

    Ah, I think I see the problem... 

     
    @Zecc said:
    if($_REQUEST['discount_price'][$key] != 0) //avoid division by 0
    {
    $value = floatval($_REQUEST['list_price'][$key]);
    $discount = - 100 + ($value * 100) / $value;
    $discount = round($discount, 2);
    }
    else
    $discount = 100;

     

    Clealy, should be named "%discount" rather than "$discount" since it's calculating the discount in percent, not dollars!



  • And lower on the same method:

    foreach($response_items as $item)

    {

        if( ! is_array($response_items) )

            $item = $response_items;

     

        // In the middle of a bunch of code where $item is used, but not $response_items :

        // (notice the missing, or perhaps misplaced, parenthesis) 

        $_ POST['discount_price'][$item->item_no] = $item->price * 1 - ($item->final_discount / 100);

     

         if( ! is_array($response_items) )

            break;

    }

     

    I guess I can get the intent behind the weird code, but were they asleep to actually think this would work?



  • Please! No more! Still trying to get over the horror of the first one! I can't feel my legs!



  •  I trust this code isn't in production anywhere. If it is maybe you can add some logic to give any user %iali% a special discount. Whatever it is I'd be buying ... does it have a good resale value?

     



  •  @Zecc said:

    Don't forget that you could still have a division by 0 if $_REQUEST['list_price'][$key] == 0    :)

    Anyway, I've corrected the code to, you know, something that actually makes sense.

     

    Edit: also, it's not a cookie, but the value of an input field, named something like "discount_price[2]". And it is set by the user anyway (we're talking back-office here).

    $_REQUEST includes Cookies, GETs, POSTS it depends on the ordering/setup in your php.ini as to what gets through in what order. No matter how you slice it, if you use $_REQUEST and don't validate where the data is coming from, the user can just submit whatever they please if they know the name of the key to name it (ie, if they can view source and just pull your JS submit code/form names).

    And my code only results in division by zero because the original code has division by zero if there is a real float value for $value. 

    $discount = - 100 + ($value * 100) / $value; 
    This -100 + ($value/$value)*100

    This is the same as -100 + ($value/$value)*100, which is the same as -100+100. That's 0.

    To be honest, I can't figure out what it was even attempting to truly put in the $discount variable... the actual $ amount of the discount, the % of discount, or a 0 or 100 based on what the user inputs for [list_price]['key']?

    It makes me cry. I'll go back to looking at code I get paid for, it hurts me less. 



  • @darkmattar said:

    $_REQUEST includes Cookies, GETs, POSTS it depends on the ordering/setup in your php.ini as to what gets through in what order. No matter how you slice it, if you use $_REQUEST and don't validate where the data is coming from, the user can just submit whatever they please if they know the name of the key to name it (ie, if they can view source and just pull your JS submit code/form names).
    If only the js code were that clear to read..  :)

    But yes, I guess you're right. But that really doesn't matter anyway, because they can give whatever discounts they want to their clients. It's a clear text input field.

    @darkmattar said:

    This is the same as -100 + ($value/$value)*100, which is the same as -100+100. That's 0.
    Actually, this is zero if $value != 0 and division by zero if $value == 0. But enough of that already.


Log in to reply