How to declare two actions in ? : format?-CodePudding

My code is as below:

if(existingWishlistItem) {
  return wishlistItems.map(wishlistItem => 
    wishlistItem.id === wishlistItemToAdd.id
    ? toast.error('This item is already in your wishlist')
    : wishlistItem
  )
}

I want this function to check if there are existing wishlist item in the array, then it pop up an error message to user and return back the wishlistItem array. But I find that I just can write one action after the '?', so are there any ways to pop up the message and return back the wishlistItem at the same time?

Thanks for help!

CodePudding user response：

It's possible to do this with the conditional operator, but it's not a good idea. It's hard to read, hard to debug, and easy to get wrong.

Instead, just use an if:

if (existingWishlistItem) {
    for (const {id} of wishlistItems) {
        if (id === wishlistItemToAdd.id) {
            toast.error('This item is already in your wishlist');
            break; // I assume the ID values are unique, so you can stop here
            // Or: `return wishlistItems;` if you don't need to make a
            // copy in this case
        }
    }
    return wishlistItems; // If you don't need to make a copy
    // Or: `return wishlistItems.slice()` if you do need to make a copy
}

(Or — again assuming id values are unique — you could use find instead of the for-of loop to find the existing item.)

For completeness, you can use the comma operator to do two things in any expression (including the operands of the conditional operator): (first, second). The comma operator evaluates its left-hand operand, throws away that result, and then evalutes its right-hand operand and takes that value as its result. Applying that to your example:

// DON'T DO THIS
if (existingWishlistItem) {
    return wishlistItems.map(wishlistItem => 
        wishlistItem.id === wishlistItemToAdd.id
            ? (toast.error('This item is already in your wishlist'), wishlistItem)
            : wishlistItem
    );
}

CodePudding user response：

This is not what map or ternaries are for.

Idiomatically, ternaries are used for conditional behavior that does not have side effects. Like return upperCase ? "HELLO" : "hello". This is because complex ternaries are hard to read and so it's hard to tell, at a glance, where the side effect is happening.

Likewise map is for transforming objects in a sequence according to some function. It's best practice for map to have no side effects, because code is easier to read when side-effects are clearly separated from data transformation.

A far more idiomatic implementation of your code would be:

if(existingWishListItem) {
    if (wishlistItems.some(x => x.id === wishlistItemToAdd.id) {
        toast.error(msg)
    }
    return wishListItems
}

CodePudding user response：

Your problem is you're using map which populates a new list of data with the same array length. If you want to find an existing item, you just simply use find. For example

if(existingWishlistItem) {
   const foundWishlistItem = wishlistItems.find(wishlistItem => wishlistItem.id === wishlistItemToAdd.id)

   if(foundWishlistItem) {
      toast.error('This item is already in your wishlist')
      //TODO: You can return or do whatever after found existing wishlist item
   }
   return wishlistItems
}

Besides that, if you want to have true/false value instead of finding an existing object, you can use some instead

if(existingWishlistItem) {
   const isFoundWishlistItem = wishlistItems.some(wishlistItem => wishlistItem.id === wishlistItemToAdd.id)

   if(isFoundWishlistItem) {
      toast.error('This item is already in your wishlist')
      //TODO: You can return or do whatever after found existing wishlist item
   }
   return wishlistItems
}

CodePudding user response：

Depends on how you define your action. You can use the || operator, and that way first expression is your alert and second is the value you return:

let x = [1,2,3];
let y = x.map((a) => a%2===0? (alert("XXx") || a) : a 1
);
console.log(y);