3
\$\begingroup\$

I implemented a shopping cart with asp.net and jquery on my website, and here is my js code which calculates amount and total sum on client side:

 $(document).ready(function () {
            update();
        $(".quant").change(function() {

            update();        
         });
        function update() {

            var id = $('.quant').attr('data-id');
            var sum = 0.0;
            var quantity;
            $('#myTable > tbody  > tr').each(function () {


                quantity = $(this).find('.quant').val();
                var price = parseFloat($(this).find('.price').attr('data-price').replace(',', '.'));
                var amount = (quantity * price);

                sum += amount;
                $(this).find('.amount').text('' + amount + ' грн');

            });

            $('.total').text(sum + ' грн');

            $.get(
             '/Cart/AddTocart',
             {
                 id: id,
                 returnUrl: '',
                 quantity: quantity

             }
         );
        }

        });

Some people review my code and said me that it needs refactoring:

  1. this part of code var id = $('.quant').attr('data-id');

    is wrong because if we have 1 element jquery works correctly, but if we have array of elements - jquery "takes" only first element and attr doesn't work good , so how can i fix it?? This id I pass to server side. I thought it needs write something like this:

    var id = $(this).attr('data-id');
    

    but id will always undefined.

  2. Problem here:

    var quantity;
          $('#myTable > tbody  > tr').each(function () {
             quantity = $(this).find('.quant').val();
             ...
    }
    

    It recalculates all elements of quantity and quantity "saves" last known value; what's wrong here??

  3. I was told that I pass first id and last quantity, but it is all present when I call function in $(document) - Please explain this to me.

    Here is html code:

    <thead>
        <tr>
            <th class="text-center">Товар</th>
            <th class="text-center">К-сть</th>
            <th>Назва Товару</th>
            <th class="text-right">Ціна</th>
            <th class="text-right">Загальна ціна</th>
        </tr>
    </thead>
    
    <tbody>
        @foreach (var line in Model.Cart.Lines)
        {
    
    
            IEnumerable<FurnitureImages> images = line.Furniture.Images;
            FurnitureImages mainImage = images.Where(x => x.IsMainImage).FirstOrDefault();
    
            <tr>
               <td class="text-center">
                   @if (mainImage != null)
                   {
    
                       <img src="@Url.Content(mainImage.Path)" style="width:110px; height:70px" />
                   }
    
               </td>
    
                <td class="text-center">
                <input type="text" data-id="@line.Furniture.FurnitureId" data-price="@line.Furniture.Price" value="@line.Quantity" class="quant" />
                </td>
                <td class="text-left">@line.Furniture.Name</td>
                <td class="text-right price"  data-price="@line.Furniture.Price">@((line.Furniture.Price).ToString("#.## грн"))</td>
                <td class="text-right amount">@((line.Quantity * line.Furniture.Price).ToString("#.## грн"))</td>
                <td>
                    @using (Html.BeginForm("RemoveFromCart", "Cart", new { Id = line.Furniture.FurnitureId }))
                    {
                        @Html.Hidden("Id", line.Furniture.FurnitureId)
                        @Html.HiddenFor(x => x.ReturnUrl)
                        <input class="btn btn-sm btn-warning" type="submit" value="Видалити з кошику" />
                    }
    
    
                </td>
    
    
            </tr>
        }
    
    </tbody>
    
    <tfoot>
    
        <tr>
            <td colspan="4" class="text-right"><b>Всього до оплати:</b></td>
            <td id="test" class="text-right total">@Model.Cart.ComputeTotalValue().ToString("#.## грн")</td>
        </tr>
    
    
    </tfoot>
    
\$\endgroup\$
  • \$\begingroup\$ What does the HTML (rendered by ASP.net) look like? Is it basically a table with each row (I.e. <tr>) containing 1 input with class "quant"? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jul 31 '17 at 14:51
  • \$\begingroup\$ @SamOnela hello, I updated my asnwer by adding HTML code , yes there is input for quantity \$\endgroup\$ – M-Misha-M Jul 31 '17 at 14:57
3
\$\begingroup\$
  1. this part of code var id = $('.quant').attr('data-id');

Presuming there are multiple inputs with class "quant" (perhaps 1 for each row in the table), then this code will get a collection of elements and then as is desribed in the documentation for .attr():

Get the value of an attribute for the first element in the set of matched elements or set one or more attributes for every matched element.

So yes it will get the value of the data-id attribute of the first element with that class name.

Consider the following snippet (try running it to see the result):

//find all elements with class name "quant"
var inputsWithClassQuant = $('.quant');
console.log('inputsWithClassQuant.length: ',inputsWithClassQuant.length);
//attr only returns attribute of first element:
console.log("inputsWithClassQuant.attr('data-id') (first element only): ",inputsWithClassQuant.attr('data-id'));
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<input type="text" data-id="30" data-price="20" value="3" class="quant" />
<input type="text" data-id="35" data-price="10" value="0" class="quant" />
<input type="text" data-id="36" data-price="15" value="2" class="quant" />

  1. Problem here:

    var quantity;
    $('#myTable > tbody  > tr').each(function () {
     quantity = $(this).find('.quant').val();
     ...
    }
    

Yes, quantity = $(this).find('.quant').val(); overwrites the previous value each time so after the loop (i.e. the call to .each()), quantity will contain the value of the last element with class name quant.

  1. I was told that I pass first id and last quantity, but it is all present when I call function in $(document) - Please explain this to me.

This is basically a summary of points #1 and #2 above.

Perhaps a better implementation would move the code to call the AJAX request (i.e. to '/Cart/AddTocart') with the id and quantity of the row that changed, then call update() to update the total. Something like the code below.

Another suggestion is to update the request sent to the server to send the entire list of items with the respective quantities. That way, if a quantity is decreased or cleared, the cart can be accurately updated. It all depends on the back-end API - i.e. if it has endpoints to add/remove/update items with quantities, etc.

I also changed the input type of the quantity inputs to "Number" - that way only numbers can be entered, and many browsers will add up/down controls to the side of the input for the user to click on.

$(document).ready(function() {
  update();
  $(".quant").change(function() {
    //this: context of the input that was changed
    console.log('calling /Cart/AddTocart; id:',$(this).attr('data-id'),' quantity: ', $(this).val());
    $.get(
      '/Cart/AddTocart', {
        id: $(this).attr('data-id'),
        returnUrl: '',
        quantity: $(this).val()
      });
    update();
  });

  function update() {
    var sum = 0.0;
    var quantity;
    $('#myTable > tbody  > tr').each(function() {

      quantity = $(this).find('.quant').val();
      var price = parseFloat($(this).find('.price').attr('data-price').replace(',', '.'));
      var amount = (quantity * price);

      sum += amount;
      $(this).find('.amount').text('' + amount + ' грн');
    });
    $('.total').text(sum + ' грн');
  }
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<table id="myTable">
  <thead>
    <tr>
      <th class="text-center">Товар</th>
      <th class="text-center">К-сть</th>
      <th>Назва Товару</th>
      <th class="text-right">Ціна</th>
      <th class="text-right">Загальна ціна</th>
    </tr>
  </thead>
  <tr>
    <td class="text-center"></td>
    <td class="text-center">
      <input type="number" data-id="97" data-price="30" value="1" class="quant" />
    </td>
    <td class="text-left">Kart</td>
    <td class="text-right price" data-price="30">30</td>
    <td class="text-right amount">30</td>
    <td>
      <input class="btn btn-sm btn-warning" type="submit" value="Видалити з кошику" /> </td>
  </tr>
  <tr>
    <td class="text-center"></td>
    <td class="text-center">
      <input type="number" data-id="99" data-price="60" value="0" class="quant" />
    </td>
    <td class="text-left">Kart</td>
    <td class="text-right price" data-price="10">10</td>
    <td class="text-right amount">30</td>
    <td>
      <input class="btn btn-sm btn-warning" type="submit" value="Видалити з кошику" /> </td>
  </tr>
  <tfoot>
    <tr>
      <td colspan="4" class="text-right"><b>Всього до оплати:</b></td>
      <td id="test" class="text-right total">@Model.Cart.ComputeTotalValue().ToString("#.## грн")</td>
    </tr>
  </tfoot>
</table>

\$\endgroup\$
  • \$\begingroup\$ okay , thanks , I have a lot of questions , so: Now I see that we get only first element with using attr , what I must to use instread of attr?? \$\endgroup\$ – M-Misha-M Jul 31 '17 at 15:09
  • \$\begingroup\$ Okay , I see your update , but one question still , I see how u send quantity to server via $(this).val() , but what about recalculation quantity in function "update" in each loop?? Leave it there?? \$\endgroup\$ – M-Misha-M Jul 31 '17 at 15:24
  • \$\begingroup\$ Well, since it is used for calculating the total, it should be kept in some form... \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jul 31 '17 at 15:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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