Refactoring complex multi condition if-else statements

Question

So Basically I have created this monstrosity which checks if the user has selected any of the filters options (user can also select multiple filters options), compares them with the values in the array and gives back an array of filtered values. It works, but it will get harder to maintain it in the future, so is there any way I could split it up into modules and make it more dynamic?

response = response.filter(job => {
      const filterCategory = job.category.id === category;
      const filterCountry = job.mainLocation.address.country === country;
      const filterZip = job.mainLocation.address.zipCode === zip;
      const filterName = findRightName(job, locale) === name;

      if (name && country && zip && category) {
        return filterCategory && filterCountry && filterZip && filterName;
      } else if (name && country && zip) {
        return filterCountry && filterZip && filterName;
      } else if (name && country && category) {
        return filterCategory && filterCountry && filterName;
      } else if (name && zip && category) {
        return filterCategory && filterZip && filterName;
      } else if (country && zip && category) {
        return filterCategory && filterCountry && filterZip;
      } else if (name && country) {
        return filterCountry && filterName;
      } else if (name && category) {
        return filterCategory && filterName;
      } else if (name && zip) {
        return filterZip && filterName;
      } else if (zip && category) {
        return filterCategory && filterZip;
      } else if (country && zip) {
        return filterCountry && filterZip;
      } else if (country && category) {
        return filterCategory && filterCountry;
      } else if (name) {
        return filterName;
      } else if (country) {
        return filterCountry;
      } else if (zip) {
        return filterZip;
      } else if (category) {
        return filterCategory;
      } else {
        return job;
      }
    });

Answer 1

Since no one seems to have mentioned it.

One can also use OR (||) for these kind of checks.

Example snippet:

function findRightName (job, locale) {
  if(job.name[locale] !== undefined)
    return job.name[locale];
  else
    return job.name['en-GB'];
};

let response = [
{
  category : { id : 1, name : 'Hats' },
  mainLocation : {
    address : { 
      country : 'Wonderland',
      zipCode : '123'
    }
  },
  name : {
    'en-GB' : 'Mad Hat'
  }
},
{
  category : { id : 2, name : 'Cards' },
  mainLocation : {
    address : { 
      country : 'Wonderland',
      zipCode : '456'
    }
  },
  name : {
    'en-GB' : 'Queen'
  }
}
];

let category = 1;
let country = 'Wonderland';
let zip = undefined;
let name = 'Mad Hat';
let locale = 'en-GB';

let filteredJobs = response.filter((obj) => {
    let filterCategory = (!category || obj.category.id === category);
    let filterCountry = (!country || obj.mainLocation.address.country === country);
    let filterZip = (!zip || obj.mainLocation.address.zipCode === zip);
    let filterName = (!name || findRightName(obj, locale) === name);
    return filterCategory && filterCountry && filterZip && filterName;
});

console.log(filteredJobs);

Answer 2

First of all, you don't need to check all possible combinations of name, country, zip, and category with their associated truth checks. You can only use the filter* variables if the variable associated with them is set. This means you have a total of four checks, instead of all possible combination of these four items.

You can just check them one by one then AND the results together:

let result = true;

if (name) {
  result = result && filterName;
}

if (country) {
  result = result && filterCountry;
} 

if (zip) {
  result = result && filterZip;
}

if (category) {
  result = result && filterCategory;
}

return result;

result starts as true because we are only doing AND operations and this is the identity element for AND. From there, all the logic is guaranteed to be correct. For example:

• if filterCountry is true, yet result is already false, then false && true produces false
• if filterCountry is false, but result is true then true && false still produces false

Thus if any of filter* is actually false, then at the end result would be guaranteed to be false.

Returning job can be factored out. Assuming job is never a falsy value, then with all of name, country, zip, and category are falsy, result is simply going to be true.

This can be shortened somewhat if you just build a table of what to check:

const checks = [
// key         |   check
  [name,     /*|*/ filterName    ],
  [country,  /*|*/ filterCountry ],
  [zip,      /*|*/ filterZip     ],
  [category, /*|*/ filterCategory],
];

return checks
  .filter(([key]) => key)     //remove any falsy keys
  .every(([, check]) => check) //check the associated filter variable

You can even fold the filter* variables into this as functions to only be executed when you need them, instead of running them all at once, which makes the entirety of the filter callback the following:

response = response.filter(job => {
  const checks = [
  //key          |   checkFn
    [name,     /*|*/ () => findRightName(job, locale) === name          ],
    [country,  /*|*/ () => job.mainLocation.address.country === country ],
    [zip,      /*|*/ () => job.mainLocation.address.zipCode === zip     ],
    [category, /*|*/ () => job.category.id === category                 ],
  ];

  return checks
    .filter(([key]) => key)           //remove any falsy keys
    .every(([, checkFn]) => checkFn()) //run the function that will do the check
}

What I'd personally do is parametrise those functions and extract them out of the .filter callback. It just makes the callback look a bit cleaner if nothing else:

const checks = [
//key          |   checkFn
  [name,     /*|*/ job => findRightName(job, locale) === name          ],
  [country,  /*|*/ job => job.mainLocation.address.country === country ],
  [zip,      /*|*/ job => job.mainLocation.address.zipCode === zip     ],
  [category, /*|*/ job => job.category.id === category                 ],
];

response = response.filter(job => checks
  .filter(([key]) => key)               //remove any falsy keys
  .every(([, checkFn]) => checkFn(job)) //run the function that will do the check
)

Answer 3

You're missing the actual condition of what you are trying to achieve. Instead of:

const filter = job.param === param;

what you really want is only fail if the param exist. So a better expression would be:

const filter = true;
if (param) filter = job.param === param;

Or in ternary expression:

const filter = param ? job.param === param : true;

Using this logic you can completely remove the if/else:

const filterCategory = category ? job.category.id === category                 : true;
const filterCountry  = country  ? job.mainLocation.address.country === country : true;
const filterZip      = zip      ? job.mainLocation.address.zipCode === zip     : true;
const filterName     = name     ? findRightName(job, locale) === name          : true;

return filterCategory && filterCountry && filterZip && filterName;

Answer 4

I would simplify the ifs this way using ternary operators:

response = response.filter(job => {
      const filterCategory = job.category.id === category;
      const filterCountry = job.mainLocation.address.country === country;
      const filterZip = job.mainLocation.address.zipCode === zip;
      const filterName = findRightName(job, locale) === name;
      let ret = job;

      ret = name ? ret && filterName : ret;
      ret = country ? ret && filterCountry : ret;
      ret = zip ? ret && filterZip : ret;
      ret = category ? ret && filterCategory : ret;

      return ret;
}