Question 35655 in Jquery Question posted in
The official Jquery Api Documentation can be found here.

Change jquery function in for loop?

Could this function be shortened?

`

$(function () {
let section1 = $('.amount_1').attr('data-product_id');
 let section2 = $('.amount_2').attr('data-product_id');
 let section5 = $('.amount_5').attr('data-product_id');
 let section10= $('.amount_10').attr('data-product_id');
 let section15 = $('.amount_15').attr('data-product_id');
 let $link1 = $("#" + section1).find('a').clone().text('');
 if ($link1.length > 0) {
   $('.col1').removeAttr("href").removeClass("single").wrap($link1).addClass("active");
}
let $link2 = $("#" + section2).find('a').clone().text('');
 if ($link2.length > 0) {
   $('.col2').removeAttr("href").removeClass("single").wrap($link2).addClass("active");
}
let $link5 = $("#" + section5).find('a').clone().text('');
 if ($link5.length > 0) {
   $('.col5').removeAttr("href").removeClass("single").wrap($link5).addClass("active");
}
let $link10 = $("#" + section10).find('a').clone().text('');
 if ($link10.length > 0) {
   $('.col10').removeAttr("href").removeClass("single").wrap($link10).addClass("active");
}
let $link15 = $("#" + section15).find('a').clone().text('');
 if ($link15.length > 0) {
   $('.col15').removeAttr("href").removeClass("single").wrap($link15).addClass("active");
}
});

`

Maybe in a for loop? I don’t really know how cause the numbers are random.
Thanks in advance

Tags:

2

Answers


  1. 0

    If you have access to change the HTML, I would personally recommend that you change it use a single class that can be gathered in a single go… and move the amount to it’s own data attribute

    For example, class="amount_1" in your HTML would become class="amount" data-amount="1" etc

    $(function () {
  $(".amount").each(function() {
    let $amount = $(this);
    let amountVal = $amount.data("amount");
    let productId = $amount.data("product_id");
    
    let $link = $(`#${productId}`).find('a').clone().text('');
    if ($link.length > 0) {
      $(`.col${amountVal}`).removeAttr("href").removeClass("single").wrap($link).addClass("active");
    }
  });
});

    Please also note jQuery gives you the .data() method so you don’t need to use attr

    Login or Signup to reply.
  2. 0

    It does not look so "random" for me, numbers 1, 2, 5, 10, 15 are pretty repetitive. Extract them and use them as a part of your jquery selectors.

    Something like this (upd, version with 1 loop only):

    $(function () {
  ["1", "2", "5", "10", "15"].forEach((x) => {
    const section = $(`.amount_${x}`).attr("data-product_id");
    const $link = $(`#${section}`).find("a").clone().text("");
    if (!$link || $link.length <= 0) return;
    $(`.col${x}`)
      .removeAttr("href")
      .removeClass("single")
      .wrap($link)
      .addClass("active");
  });
});
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search