1
\$\begingroup\$

Haven't worked too much with jQuery and I had to get the values from different elements (they have the ids 0, 1 and 2) in order to compute the sum of them and update a field with that value.

The code works well but I was wondering if it is possible to write less code and use something like a for loop in this case.

Here is the code:

function total() {
  var subtotal0 = parseFloat(
    $('#0').find('.cart-subtotal').text().replace('€', '')
  );

  var subtotal1 = parseFloat(
    $('#1').find('.cart-subtotal').text().replace('€', '')
  );

  var subtotal2 = parseFloat(
    $('#2').find('.cart-subtotal').text().replace('€', '')
  );

  var total = subtotal0 + subtotal1 + subtotal2;

  $('.cart-total-price').text(total + '€');
}

Is it possible to make it better?

\$\endgroup\$
2
\$\begingroup\$

Instead of IDs (like #0), use classes instead. (Numeric-indexed IDs are quite a code smell anyway.) Or, if you can't remove the IDs entirely, just add classes to those elements. For example, adding the class of subtotal-container to each of those elements with IDs.

To make things clearer, I'd also recommend calling the function something like calculateTotal instead of total - it's more precise and also avoids possibly-confusing shadowing:

function calculateTotal() {
  const total = [...$('.subtotal-container .cart-subtotal')]
    .map(subtotalElm => 
  Number(subtotalElm.textContent.replace('€', '')))
    .reduce((a, b) => a + b, 0);
  $('.cart-total-price').text(total + '€');
}

Though, at this point, you may as well remove the dependency on jQuery entirely if you wished, it's not really accomplishing anything useful:

const total = [...document.querySelectorAll('.subtotal-container .cart-subtotal')]
  .map(subtotalElm => Number(subtotalElm.textContent.replace('€', '')))
  .reduce((a, b) => a + b, 0);
document.querySelector('.cart-total-price').textContent = total + '€';

With jQuery, keep in mind that

$('someSelector').find('someOtherSelector')

is equivalent to

$('someSelector someOtherSelector')

via the descendant selector.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I was writing it as a function in order to be able to call it and I see that you've done it as an array. In this case it doesn't work anymore. Is it possible to make it "callable" or should it be changed also where is called from total() to something else? \$\endgroup\$ – Leo Messi Sep 2 at 14:36
  • \$\begingroup\$ I was trying to only write the necessary code, thought it was obvious that it was the new content of the total function \$\endgroup\$ – CertainPerformance Sep 2 at 14:41

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.