1

I have several input checkbox, for example
5 <input type="checkbox" name="A" class="A"> (with different value)
4 <input type="checkbox" name="B" class="B"> (with different values)
6 <input type="checkbox" name="C" class="C"> (with different values)
and 3 arrays array_A[], array_B[], array_C[] to store input's value every time it's checked or not.

I wrote a function like that to add and remove input's value to the right array

$('input[type="checkbox"]').on('change', function(){
  if($(this).is(':checked')){
    if($(this).is('.A')){
      array_A.push($(this).val());
    }else if($(this).is('.B')){
      array_B.push($(this).val());
    }else if($(this).is('.C')){
      array_C.push($(this).val());
    }
  }else{
    if($(this).is('.A')){
      position = array_A.indexOf($(this).val());
      if ( ~position ) array_A.splice(position, 1);
    }else if($(this).is('.B')){
      position = array_B.indexOf($(this).val());
      if ( ~position ) array_B.splice(position, 1);
    }else if($(this).is('.C')){
      position = array_C.indexOf($(this).val());
      if ( ~position ) array_C.splice(position, 1);
    }
  }
})

I currently use if else function so many time and I definitely don't think it's a good way to do that, I wonder if is there a better way to do that. Thank you.

Luu Hoang Bac
  • 433
  • 6
  • 17

1 Answers1

1

Consider using an object indexed by the class name instead:

const arrays = {
  A: [],
  B: [],
  C: [],
}
$('input[type="checkbox"]').on('change', function(){
  const arr = arrays[this.className];
  if($(this).is(':checked')){
    arr.push(this.value);
  } else {
    const position = arr.indexOf(this.value);
    if (position !== -1) {
      arr.splice(position, 1);
    }
  }
});

Note that it's probably better to use !== -1 rather than ~ - most readers of the code will likely be quite confused by ~ until they look it up.

CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • It worked, thank you. By the way, at first, I saw you use map() to solve this problem, but you changed your code right after. Is it because map() is way more complicated than the current way or something? – Luu Hoang Bac Mar 14 '20 at 08:25
  • 1
    I initially thought you had 3 separate inputs, and a Map allows you use use objects as "keys". But then I realized that you meant that you had a bunch of inputs with the same name/class, not just one per, so a Map wouldn't work. – CertainPerformance Mar 14 '20 at 08:26
  • excuse me, now I want to convert array A[], B[], C[] to string (by using `A_str = A.join()`). Is there any other way that I can do this without creating a new variable (such as A_str, B_str,...). Note: the final result a want is strings which are joined by the arrays, so I don't need to have arrays anymore. – Luu Hoang Bac Mar 14 '20 at 10:10
  • 1
    I'd keep the array, since it'll make manipulation of it much easier. *When* you want to turn one of them into a string, call `.join` on it, `const a_str = arrays.A.join();` Or if you don't want to declare a new variable, just use the expression `arrays.A.join();` wherever it's needed – CertainPerformance Mar 14 '20 at 10:12