Question 108798 in Javascript Question posted in
A very good W3school tutorial can be found here.

Javascript – How to refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed?

when I add this check:
card.targetUrl !== "/test"

I get this error and I cannot commit, how do I reduce this complexity?

 const link = useMemo(() => {
    if (!card?.targetUrl || card.targetUrl === "/test") {
      return "/account";
    } else {
      return card.targetUrl;
    }
  }, [card]);

this error is not present when the code looks like this:

const link = useMemo(() => {
    if (card?.targetUrl) {
      return card.targetUrl;
    } else {
      return "/account";
    }
  }, [card]);
Tags:

3

Answers


  1. 0

    Cognitive Complexity can be defined as "a measure of how difficult a unit of code is to intuitively understand".

    To me, the intent of your code as it is now is not very obvious, since I need to mentally compute the null check, the negation, the or and the equality on the right side. Doesn’t seem like much when we’re writing it, but others (and even you later) might find it difficult to read, hence the high cognitive complexity score.

    I have some suggestions for you in order to make it simpler to understand:

    1. Have each condition in a separate if clause:

      const link = useMemo(() => {
  if (card?.targetUrl == null) {
    return "/account";
  }
  if (card.targetUrl === "/test") {
    return "/account";
  }
  return card.targetUrl;
}, [card]);

      Some people might be hesitant to repeat the same return value (remember DRY?), but making your code easier to read and easier to change makes it worthwhile. For instance, maybe you’ll want to remove the card.targetUrl === "/test" condition at a later time, in the sample code above it would be easy because they’re separate.

    2. Don’t use the else keyword.

      Since your code is doing early returns, the else clause is not needed. This advice comes from Object Calisthenics, check it out.

    3. If you don’t want to repeat returns, there’s another way to make the code more readable by extracting the condition into a new function and giving it a descriptive name (pardon my rusty Javascript):

      const link = useMemo(() => {
  if (shouldUseDefaultUrl(card)) {
    return "/account";
  }
  return card.targetUrl;
}, [card]);

function shouldUseDefaultUrl(card) {
  if (card?.targetUrl == null) {
    return true;
  }
  if (card.targetUrl === "/test") {
    return true;
  }
  return false;
}
    Login or Signup to reply.
  2. 0

    Cognitive complexity is increased when there are breaks in the linear flow of control. if, else blocks and early returns may increase it. You can remove one inner block and the early returns by initialising a variable with the default value, like this:

    const link = useMemo(() => {
    let url = card?.targetUrl;
    if (!url || url === "/test") {
       url = "/account";
    }
    return url;
  }, [card]);
    Login or Signup to reply.
  3. 0

    Just remove useMemo. It takes more CPU time than calculations inside of it.

    const link = !card || card.targetUrl === "/test" ? "/account" : card.targetUrl

    with useMemo each render you have to create a functuion, get previously stored dependencies, loop through it, and even if there were no changes you already spent ten times more CPU time that simple string compare.
    And if card was chnaged, then… uhh. execute the function, store return value somewhere, replace dependencies.. ohh.. and store hook into hook execution chain… I believe even more overhead is here.

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search