refactor/simplify multiple if conditionals in javascript

Question

On a webpage there are multiple select tags that hides or shows table rows.

The logic is implemented in javascript with multiple if/else if - conditionals and I'm looking for a way to simplify it:

if(
  (validPublication == true)
  && (productPublication > 0)
  && (genre == -1 || $(this).data('genre') == genre)
  && identificator_shared == false
  && (format == 'foo' && $(this).data('foo') == true)
  && (identificator_version == -1 || $(this).data('topic_version') == identificator_version)
) {
  $('#filter_topic_version_eq').removeAttr('disabled');
  $('#filter_identificator_version_eq').removeAttr('disabled');
  $('#filter_genre_eq').removeAttr('disabled');
  $('#filter_format_eq').removeAttr('disabled');
  $(this).show();
  if ($(this).data('product')) {
    products.push($(this).data('product'));
  }
}
else if(
  (validPublication == true)
  && (productPublication > 0)
  && (genre == -1 || $(this).data('genre') == genre)
  // if format foo && drvt_shared=true dann alle Versionen anzeigen
  && (format == 'foo' && (identificator_shared == true))
  && ($(this).data('foo') == true)
) {
  $('#filter_topic_version_eq').removeAttr('disabled');
  $('#filter_identificator_version_eq').removeAttr('disabled');
  $('#filter_format_eq').removeAttr('disabled');
  $('#filter_genre_eq').removeAttr('disabled');
  $(this).show();
  if ($(this).data('product')) {
    products.push($(this).data('product'));
  }
}
else if(
  (validPublication == true)
  && (productPublication > 0)
  && (genre == -1 || $(this).data('genre') == genre)
  && (format == 'bar' && (identificator_shared == false))
  && ($(this).data('bar') == true)
  && (identificator_version == -1 || $(this).data('topic_version') == identificator_version)
) {
  $('#filter_format_eq').removeAttr('disabled');
  $('#filter_genre_eq').removeAttr('disabled');
  $('#filter_topic_version_eq').removeAttr('disabled');
  $('#filter_identificator_version_eq').removeAttr('disabled');
  $(this).show();
  if ($(this).data('product')) {
    products.push($(this).data('product'));
  }
}
... and so on

I don't think using switch statements would be a huge improvement.

=== UPDATE: In the comments the advice to extract the if condition in a separate function was given.

Instead of

  if (validPublication == true)
    && (productPublication > 0)
    && (genre == -1 || $(this).data('genre') == genre)
    && identificator_shared == false
    && (format == 'foo' && $(this).data('foo') == true)
    && (identificator_version == -1 || $(this).data('topic_version') == identificator_version)
  )

I should use:

function foo(validPublication, productPublication, genre,identificator_shared,format,identificator_version) {
  if(
    (validPublication == true)
    && (productPublication > 0)
    && (genre == -1 || $(this).data('genre') == genre)
    && identificator_shared == false
    && (format == 'foo' && $(this).data('foo') == true)
    && (identificator_version == -1 || $(this).data('topic_version') == identificator_version)
  ) {
    return true
  }
}

so the main conditional looks like this:

if foo(validPublication, productPublication, genre,identificator_shared,format,identificator_version)
else if(bar())
else if(baz())

Do I understand it correctly?

One of options if to extract the complex statement to a separate function and name it so that it's clear what your intention is. The if becomes then if (somethingUnderstandable(x,y,z)) ... else if (somethingElse()) .... — Wiktor Zychla
– Wiktor Zychla, Commented Aug 25, 2020 at 7:33
Nesting things could also helps, it looks like you have common conditions in each of your branch. You could add an if for this conditions — Axnyff
– Axnyff, Commented Aug 25, 2020 at 7:43
@WiktorZychla please see my updated post. I'm not sure if I understand it correctly — Sandra Cieseck
– Sandra Cieseck, Commented Aug 25, 2020 at 8:03

Refat Alsakka · Accepted Answer · 2020-08-25 09:02:51Z

i did my best to reduce the code, it can be much better but it will be complex.

you should read more about DRY (Don't Repeat Yourself) it wil help you so much to avoid that and write simple readable code.

This aricle may help you

function isProductPublication(productPublication) {
  return productPublication > 0;
}

function isgenre(elm) {
  return genre == -1 || $(elm).data('genre') == genre;
}

function isEqual(check, value) {
  return check == value;
}

function _do(elm) {
  $('#filter_topic_version_eq').removeAttr('disabled');
  $('#filter_identificator_version_eq').removeAttr('disabled');
  $('#filter_genre_eq').removeAttr('disabled');
  $('#filter_format_eq').removeAttr('disabled');
  $(elm).show();
  if ($(elm).data('product')) {
    products.push($(elm).data('product'));
  }
}


if (
  (isEqual(validPublication, true) && isProductPublication(productPublication) && isgenre(this))
  && 
  (
    (isEqual(identificator_shared, false) && (isEqual(format, 'foo') && isEqual($(this).data('foo'), true)) && (isEqual(identificator_version, -1) || isEqual($(this).data('topic_version'), identificator_version)))
    || (isEqual(format, 'foo') && isEqual(identificator_shared, true) && isEqual($(this).data('foo'), true))
    || (isEqual(format, 'bar') && isEqual(identificator_shared == false) && isEqual($(this).data('bar'), true) && (isEqual(identificator_version, -1) || isEqual($(this).data('topic_version'), identificator_version)))
  )
) {
  _do(this);
}

Collectives™ on Stack Overflow

refactor/simplify multiple if conditionals in javascript

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest