How to refactor and reduce complexity of this code

Question

code climate tells me this block of code has a Cognitive Complexity of 7 (exceeds 5 allowed). Any ideas how I would refactor this to reduce complexity?

const validateDays = (param, days, dateValues) => {
  if (!days || !param) {
    return;
  } else if (param === 'from' && dateValues) {
    const disabledDays = {
      before: days.before,
      after: parse(dateValues['to'])
    };
    return { disabledDays };
  } else if (param === 'to' && dateValues) {
    const disabledDays = {
      before: parse(dateValues['from']),
      after: days.after
    };
    return { disabledDays };
  } else {
    return { days };
  }
};

Answer 1

Complexity

Any code metric associated with readability is a subjective quantity. Cognitive complexity is one such metric and should be viewed with a good dose of skepticism.

Personally (to make up a metric) I would give your code a value of 1 where values over 0 are bad.

Why does it get 1? Because you use the same clause 2 times, which is 1 too many.

The value dateValues is checked twice

  } else if (param === 'from' && dateValues) {
      ...
  } else if (param === 'to' && dateValues) {
      ...

Checking the value only once means no redundancy

  } else if (dateValues) {
      if (param === 'from') {
          ...
      } else if (param === 'to') {
          ...
      }
  }

Cognitive Complexity

Which in my book is a dumber version of cyclic complexity.

Both metrics (cyclic and cognitive) can be reduced not by reducing complexity, but by tricking the parser into thinking the code is less complex.

Thus the following will likely get a lesser score than your original.

const validateDays = (param, days, dateValues) => {
    const dirs = {
        get from: () => ({before: days.before, after: parse(dateValues.to)}),
        get to: () => ({before: parse(dateValues.from, after: days.after)}), 
    };
    return days && param ? (dateValues ? dirs[param] ?? {days} : {days}) : undefined;  
}

However there are many here that consider nested ternaries as extremely complex (not my view).

Code quality

Personally I consider the bugs per line multiplied by line count the best metric for code quality.

Bugs per lines is a quantity defined by each coder. New coders have high values and this value goes down as you gain experience. The actual value is very hard to know, hence we can revert to the scalar line count as a good approximation. On average coders have a bug per line of ~ 1/100

Reducing line count is the best thing new coders can do to increase code quality.

Thus using the if else style your code can be

const validateDays = (param, days, dateValues) => {
  if (!(days && param)) { return }
  var res = {days};
  if (dateValues) {
    if (param === "from") {
      res = {disabledDays: {before: days.before, after: parse(dateValues.to)}};
    } else if (param === "to") {
      res = {disabledDays: {before: parse(dateValues.from), after: days.after}};
    }
  }
  return res;
}

Has a line count of 12 as opposed to your 19 lines, and may also get a lower cognitive complexity score.

or

const validateDays = (param, days, dateValues) => {
  if (!(days && param)) { return }
  var res = {days};
  if (dateValues) {
    param === "from" &&
      (res = {disabledDays: {before: days.before, after: parse(dateValues.to)}});
    param === "to" &&
      (res = {disabledDays: {before: parse(dateValues.from), after: days.after}});
  }
  return res;
}

Answer 2

Since every if statement also has a return, you don't need the else, you can just start the next if without typing else.

This reduces the "indentation" and keeps the code "flat" (for lack of better words) and I think this would reduce the complexity, although I'm not familiar with "Code climate".

I think the nested if-else-if-else structure actually goes one level deeper each time, causing the warning that you have.