Vrftjkry

I have a function in a project that I feel could be written better. The purpose of the function is to return the direction of an arrow based on two parameters.

• 
  positiveDirection: The direction for a positive result. The values could be increasing or decreasing.


• 
  changeType: Whether the result was positive, negative or no-change.

I guess it's the if statement that is bothering me the most; it looks like it could be reduced based on some logic gate that I don't know of.

  getArrowDirection = (positiveDirection, changeType) => {



    let direction = null



    if(changeType === 'no-change') {

      return direction

    }



    if(

        (changeType === 'positive' && positiveDirection === 'increasing') ||

        (changeType === 'negative' && positiveDirection === 'decreasing')

      ) {

      direction = 'up-arrow'

    } else if(

        (changeType === 'positive' && positiveDirection === 'decreasing') ||

        (changeType === 'negative' && positiveDirection === 'increasing')

      ) {

      direction = 'down-arrow'

    }



    return direction

  }

If changeValue can only be (after your early check for 'no-change') equal to 'positive' or 'negative' and positiveDirection can only be equal to 'increasing' or 'decreasing, your if should cover all possible ways.

But you can simplify it a lot making use of the xor operator:

 direction = (changeType === 'positive' ^ positiveDirection === 'decreasing')

             ? 'up-arrow'

             : 'down-arrow';

Or make the whole function a one-liner:

return (changeType !== 'no-change')

       ? ((changeType === 'positive' ^ positiveDirection === 'decreasing') ? 'up-arrow' : 'down-arrow')

       : null;

The test for if(changeType === 'no-change') { is redundant and not needed as it will fall through if the other tests fail and return null anyways.

You could also break the statement up returning the result as needed and falling through if they fail for null.

const getArrowDirection = (dir, type) => {

    if (type === "positive") {

        if (dir === "increasing") { return "up-arrow" }

        if (dir === "decreasing") { return "down-arrow" }

    }else if (type === "negative") {

        if (dir === "increasing") { return "down-arrow" }

        if (dir === "decreasing") { return "up-arrow" }

    }

    return null;

}

Assuming that you have given all possible values you can make assumptions and reduce the code further

const getArrowDirection = (dir, type) => {

    if (type !== 'no-change') {

        if (type === "positive") { return dir === "increasing" ? "up-arrow" : "down-arrow" }

        return dir === "increasing" ? "down-arrow" : "up-arrow";

    }

    return null; // returning null is not the best. Returning undefined would

                 // be better and would not need this line

}

You can use an object as a lookup using the combined strings.

const getArrowDirection = (() => {

    const directions = {

        positiveincreasing: "up-arrow",

        negativedecreasing: "up-arrow",

        positivedecreasing: "down-arrow",

        negativeincreasing: "down-arrow",

    };

    return (dir, type) => directions[type + dir] ? directions[type + dir] : null;

})();

We don't need the direction variable - we can simply return at the appropriate point.

Once you know that changeType is one of 'positive' or 'negative' and that positiveDirection is either 'increasing' or 'decreasing', you can test whether the two equality tests match:

// Winging it, because this isn't my language!

getArrowDirection = (positiveDirection, changeType) => {



    if (changeType != 'positive' && changeType != 'negative') {

        return null;

    }

    if (positiveDirection != 'increasing' && positiveDirection != 'decreasing')

        return null;

    }



    if ((changeType === 'positive') == (positiveDirection === 'increasing')) {

        return 'up-arrow';

    else

        return 'down-arrow';

    }

  }

You could could extract your conditions into functions with very clear condition description names. Would cut down the code because many of those conditions are repeated.