How do I make this function reusable?

Question

How do I make this function reusable by passing 2 parameters, totalPrice and currentPrice.

This is the code (it works perfectly, but I want this to be in a separate function so I could use it later on):

$(".btn-quantity").on('click', function () {

    var $button = $(this);
    var oldValue = $button.parent().find("input").val();
    var totalPrice = document.querySelector(".table-price-total"); // I need this querySelector to be the function's first argument
    var currentPrice = document.querySelector(".table-price-current"); // And this the second argument 
    var newVal = parseFloat(oldValue);
    if ($button.hasClass("btn-quantity--plus")) {
      newVal++;
    } else {
      if (newVal - 1 > 0) {
        newVal--;
      }

    }
    let result = newVal * parseFloat(currentPrice.innerHTML);

    totalPrice.innerHTML = result.toFixed(2);

    $button.parent().find("input").val(newVal);

  })

The error I currently get is: Uncaugh TypeError: $btn.parent is not a function

This is what I've tried:

let $btn = document.querySelector(".btn-quantity");
  let btnFunc = (currentPrice, totalPrice) => {
    $btn = document.querySelector(".btn-quantity");
    let oldValue = $btn.parent().find("input").val();
    let cPrice = document.querySelector(currentPrice);
    let tPrice = document.querySelector(totalPrice);
    let newVal = parseFloat(oldValue);
    if ($btn.hasClass("btn-quantity--plus")) {
      newVal++;
    } else {
      if (newVal - 1 > 0) {
        newVal--;
      }
    }

    let result = newVal * parseFloat(cPrice.innerHTML);
    tPrice.innerHTML = result.toFixed(2);
    $btn.parent().find("input").val(newVal);
  }
  $btn.addEventListener('click', (event) => {
    event.preventDefault();
    btnFunc(".table-price-current", ".table-price-total");
  })

Have you tried creating your function and adding those values as parameters? What didn't work? — David, Mar 13 '20 at 14:20
If you're using jQuery anyway, in my opinion it's messy to mix native DOM APIs in with the code. Instead of `.querySelector()` I'd just use the jQuery equivalent. Less typing too. — Pointy, Mar 13 '20 at 14:23
You're getting the error because your code just calls your function directly. When an event handler function is called by jQuery, the library makes sure that `this` is bound to the element involved. — Pointy, Mar 13 '20 at 14:25

score 2 · Accepted Answer · answered Mar 13 '20 at 14:22

Generally you'd do that by using a function that makes other functions:

function makePriceCalculator(totalPrice, currentPrice) {
  return function() {
    var $button = $(this);
    var oldValue = $button.parent().find("input").val();
    var newVal = parseFloat(oldValue);
    if ($button.hasClass("btn-quantity--plus")) {
      newVal++;
    } else {
      if (newVal - 1 > 0) {
        newVal--;
      }

    }
    let result = newVal * parseFloat(currentPrice.innerHTML);

    totalPrice.innerHTML = result.toFixed(2);

    $button.parent().find("input").val(newVal);
  };
}

Then your posted example would be written as

$(".btn-quantity").on('click', makePriceCalculator(document.querySelector(".table-price-total"), document.querySelector(".table-price-current"));

Exactly ... and the function would be placed in a ` – Mike Robinson Mar 13 '20 at 14:23 — Mike Robinson, Mar 13 '20 at 14:23

How do I make this function reusable?

1 Answers1