1
\$\begingroup\$

I'd really appreciate if any of you could do a code review of my Sopping cart demo. I was wondering whether this could be improved? I'm honestly trying to improve myself too!

There's a small problem on the product list! looks like doest work if has more than 1 product. Demo: http://plnkr.co/edit/jLk0VdSgGYPwLhNKkcs7

var module =  {

    init: function () {
        this.shoppingCart();
    },

  shoppingCart: function () {
      numberIncrementer(this);
      updateQuantity(this);
      recalculateCart();

      /* Set rates + misc */
      var taxRate = 0.20;
      var fadeTime = 300;


      /* Assign actions */
      $('.product-quantity input').change( function() {
        updateQuantity(this);
      });

      $('.product-removal button').click( function() {
        removeItem(this);
      });


      /* Recalculate cart */
      function recalculateCart() {
        var subtotal = 0;

        /* Sum up row totals */
        $('.product').each(function () {
          subtotal += parseFloat($(this).children('.product-line-price').text());
        });

        /* Calculate totals */
        var tax = subtotal * taxRate;
        var total = subtotal + tax;

        /* Update totals display */
        $('.totals-value').fadeOut(fadeTime, function() {
        $('#cart-subtotal').html(subtotal.toFixed(2));
        $('#cart-tax').html(tax.toFixed(2));
        $('#cart-total').html(total.toFixed(2));
          if(total == 0){
            $('.checkout').fadeOut(fadeTime);
          }else{
            $('.checkout').fadeIn(fadeTime);
          }
          $('.totals-value').fadeIn(fadeTime);
        });
      }


      /* Update quantity */
      function updateQuantity(quantityInput) {
        /* Calculate line price */
        var quantityInput = $('.product-quantity input');
        var productRow = $(quantityInput).parent().parent();
        var price = parseFloat(productRow.children('.product-price').text());
        var quantity = $(quantityInput).val();
        var linePrice = price * quantity;

        /* Update line price display and recalc cart totals */
        productRow.children('.product-line-price').each(function () {
          $(this).fadeOut(fadeTime, function() {
            $(this).text(linePrice.toFixed(2));
            recalculateCart();
            $(this).fadeIn(fadeTime);
          });
        });  
      }

      /* Remove item from cart */
      function removeItem(removeButton) {
        /* Remove row from DOM and recalc cart total */
        var productRow = $(removeButton).parent().parent();
        productRow.slideUp(fadeTime, function() {
        productRow.remove();
        recalculateCart();
        });
      }

    function numberIncrementer(){
          $("div.product-quantity").append('<div class="inc button">+</div><div class="dec button">-</div>');

          $(".button").on("click", function() {
            var $button = $(this);
            var oldValue = $button.parent().find("input").val();

            if ($button.text() == "+") {
              var newVal = parseFloat(oldValue) + 1;
            } else {
               // Don't allow decrementing below zero
              if (oldValue > 0) {
                var newVal = parseFloat(oldValue) - 1;
                } else {
                newVal = 0;
              }
              }
            $button.parent().find("input").val(newVal);
            updateQuantity();
          });

        }
    }
}



$(document).ready(function () {
    module.init();
});
\$\endgroup\$

closed as off-topic by Jamal Jul 15 '17 at 20:27

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Jamal
If this question can be reworded to fit the rules in the help center, please edit the question.

1
\$\begingroup\$

This code style is not very good, becouse u declare functions inside a function. The problem with this is that you recreate all this functions every time u run "shoppingCart" function.

Instead try using this pattern;

(function(){

    //my private functions delare

    function recalculateCart(){
        ...
    }


    // my module public methods
    var myModule = {
        init: ...,
        shoppingCart: ...
    }

    return myModule;

})()
\$\endgroup\$
  • \$\begingroup\$ would u be able to put together a fiddle or a demo please? \$\endgroup\$ – user2505665 Sep 9 '13 at 17:36
  • \$\begingroup\$ np i need few hours to do that... going out for launch ^^ \$\endgroup\$ – mikrowelt Sep 10 '13 at 9:16

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