Javascript / React data mapper

Question

My solution to implement a status (string) from combining two other ones. I need to declare a function which takes two params (two strings) and needs to return another one based on the combination of those two strings. For ex:

carStatus(status, secondaryStatus) => string

where secondaryStatus can have multiple options. I'm using an if/else if statement which returns a third status which I need. For ex when status is 'OPEN' and secondaryStatus is 'payment1' or 'payment2' or 'payment3', the function must return a new string (status) like 'CONFIRMED'. So, my solution at this moment would be something like this:

carStatus = (status, secondaryStatus) => {
  if(status === 'OPEN' && (secondaryStatus === 'payment1' || 'payment2' || 'payment3')){
    return 'CONFIRMED';
  } else if(status === 'CANCELLED' && (secondaryStatus === 'payment4' || 'payment5' || 'payment6')){
    return 'REMOVED';
  } else if(status === 'REVIEW' && (secondaryStatus === 'payment2' || 'payment5' || 'payment5')){
    return 'CHECKED';
  }
}

<div>carStatus('OPEN', 'payment1')</div>

In div must be rendered 'CONFIRMED'.

In my implementation, I'll have to write I think other 5 else if statements.. so maybe there is a cleaner way to implement this.

Any help will be appreciated.

Answer 1

Condition bug Your current implementation has a bug. Your lines like these have the problem:

(secondaryStatus === 'payment1' || 'payment2' || 'payment3')

JavaScript only has unary operators, binary operators, and a single ternary operator. All the operators used above are binary operators; two expressions will be evaluated into a single expression until a single one is left. Since === has higher operator precedence than ||, your code is equivalent to:

(secondaryStatus === 'payment1' || 'payment2' || 'payment3')

(trueOrFalse || 'payment2' || 'payment3')

The payment2 and payment3 are not compared against secondaryStatus, and if secondaryStatus is not payment1, the whole expression will evaluate to 'payment2', a truthy value (because || will evaluate to the second value if the first is falsey):

// || operates left-to-right:

(trueOrFalse || 'payment2' || 'payment3')

((false || 'payment2') || 'payment3')

(('payment2') || 'payment3')

// payment2 is truthy, so the `||` evaluates to it:
('payment2')

To fix the logic, use an array instead, and check the status against each element in the array.

Typo? You have (secondaryStatus === 'payment2' || 'payment5' || 'payment5'), with payment5 repeated twice. Did you mean something else, like payment9?

Names carStatus does not contain the car's status, as it would sound to contain; it's a function that, when called, returns the car's status. It also takes a confusingly-similarly named status parameter. Call them something else, if at all possible: perhaps getCarStatus and whatever more specific thing the status parameter represents. Maybe call secondaryStatus: paymentType.

DRY You also want to make the code more DRY, which fits in well with the idea of using arrays instead. You might use an object, whose keys are the status that's required, and whose values are the possible secondaryStatus values as well as the return value if the secondaryStatus is found.

Also, since all the payment strings end in a number, and that number is the only thing that changes, use that number to set up the config object instead of the full payment strings:

const statusOptions = {
    OPEN:      { paymentNumbers: [1, 2, 3], status: 'CONFIRMED' },
    CANCELLED: { paymentNumbers: [4, 5, 6], status: 'REMOVED' },
    REVIEW:    { paymentNumbers: [2, 5], status: 'CHECKED' },
};
// ...
getCarStatus = (status, paymentType) => {
    const possibleOption = statusOptions[status];
    if (possibleOption) {
        const paymentNumber = Number(paymentType.match(/\d+$/)[0]);
        if (possibleOption.paymentNumbers.includes(paymentNumber)) {
            return possibleOption.status;
        }
    }
}

Live snippet:

const statusOptions = {
    OPEN:      { paymentNumbers: [1, 2, 3], status: 'CONFIRMED' },
    CANCELLED: { paymentNumbers: [4, 5, 6], status: 'REMOVED' },
    REVIEW:    { paymentNumbers: [2, 5], status: 'CHECKED' },
};
// ...
getCarStatus = (status, paymentType) => {
    const possibleOption = statusOptions[status];
    if (possibleOption) {
        const paymentNumber = Number(paymentType.match(/\d+$/)[0]);
        if (possibleOption.paymentNumbers.includes(paymentNumber)) {
            return possibleOption.status;
        }
    }
};

console.log(
  getCarStatus('OPEN', 'payment3'),
  getCarStatus('REVIEW', 'payment2')
);

Component type It looks like you're using a class component. React recommends that you try out using functional components in new code; they say they're a bit easier to work with and understand than class-based components in most circumstances, and I agree. Try using functional components instead, if you haven't already, you might like them.