Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm using JQuery and Jquery UI (latest versions).

I have a list of checkboxes styled using JQuery UI as buttons. Each have a data-price attributem containing a price in this format: data-price="40.00", data-price="25.00" etc.

When the user "checks" a box, I am adding it's data-price to the totalPrice var. Every time another box is clicked, it's own data-price is added to the total. If the user unchecks a boxm, that value is taken away. I have tried to prevent the value from going under 0.00 as well.

I then output the totalPrice into a div - totalBox.

Here's my code:

<script type="text/javascript">
    totalPrice = 0.00;
    $(function() {
        var totalBox = $('#totalBox');

        $( ".styled" ).button().bind('click', function() {
            var packagePrice = $(this).attr('data-price');
            var cost = parseFloat(packagePrice);

            if(totalPrice>=0.00) {
                if($(this).is(':checked')) {
                    totalPrice += cost;
                } else {
                    totalPrice -= cost;
                }
            }

            totalBox.html('<span class="total">Total:</span>
                           <span class="price">&pound;' 
                           + totalPrice.toFixed(2) + '</span>'
            );
        });
    });
</script>

I imagine there are some optimisations here - any thoughts?

share|improve this question

1 Answer

up vote 2 down vote accepted

When working with prices, then in my opinion it's usually a good idea to avoid floating point numbers. Binary rounding errors can easily strike unexpectedly any time.

Instead I'd suggest to work with integers (thus pennies) internally, and just add the decimal point for output.

An other unrelated point: I would move the hard-coded HTML in the script to the HTML document and only write the price.

share|improve this answer
Good point about the integers, and I also agree with the HTML now that you mention it, thank you. – Calum Jun 30 '11 at 12:47

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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