CODEWITHSUNDEEP
javascript
Sandra Cieseck
Sandra Cieseck

Reputation: 331

refactor/simplify multiple if conditionals in javascript

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?

Upvotes: 1

Views: 1083

Answers (1)

Refat Alsakka
Refat Alsakka

Reputation: 561

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);
}

Upvotes: 1

Related Questions