Display toast after action

Question

I have a React functional component where users can add a movie from a list to their dashboard. I'm displaying a toast (Material UI Snackbar) to show the result. The result can be good: movie has been added, or bad: movie is already added to your watchlist.

const addMovie = (movie: IMovie) => {
  const isMovieDuplicate = !checkForDuplicate(movies, movie);
  if (isMovieDuplicate) {
    movies.push(movie);
    const sortedMovieList = sortMovieList(movies);
    setMovies(sortedMovieList);
    addMovieToLocalStorage(sortedMovieList);
  }
  const message = isMovieDuplicate ?
    movie.original_title + ' has been added to your watchlist.' :
    movie.original_title + ' is already added to your watchlist.';
  const variant = isMovieDuplicate ? {variant: 'success'} : {variant: 'warning'};
  displaySnackbar(message, variant);
};

const displaySnackbar = (message: string, type: any) => {
  enqueueSnackbar(message, type);
};

const sortMovieList = (movies: IMovie[]) => {
  return orderBy(movies, [(movie: IMovie) =>
    returnSortType(movie, sortConfig.selectedSortType)], [sortConfig.orderType ? 'asc' : 'desc'],
  );
};

First I call a function that checks the if the current movie already exists in the movieList array. If so it returns a true. The isMovieDuplicate takes the opposite value, so in case of there being a duplicate it will be false so that the if statement is not executed.

In the if statement I push the movie object in the movies array. Then I use a function to return an sorted array. Users can specify how they want to sort the list based on title, release date.

I use the sortedMovieList value to call my React hook setMovies and put he array in the React state. Then I use that same array and put the data in my local storage. At some point this local storage will be replaced with an actual database.

When the if statement is completed I want to show the result in a snackbar (toast). Based on the isMovieDuplicate variable I create the context for the toast and pass that to the displaySnackbar function which renders it on the page.

This all works, but it doesn't feel "clean". There's quite some code, maybe I could move the code for creating the message into the displaySnackbar method?

Answer 1

Not as clean as it could be, some code duplication, and there's also some state mutation in your addMovie function.

Issues

1. movies.push(movie); mutates the current state object.
2. checkForDuplicate isn't a clear name, i.e. checking for duplicates is clear, but what is the return value?
3. Using isMovieDuplicate as the negation of the result from checkForDuplicate is completely counter-intuitive and confusing.
4. When adding data to current state and computing the next state, a functional state update should really be used.
5. Ternary logic could be reduced/simplified.
6. Handle side-effect of persisting to local storage in the component as an "effect" of updating the movies state.

Suggestions

1. Use a functional state update and shallow copy of current state in order to not mutate current state and correctly enqueue state updates.
2. Change checkForDuplicate to checkIsDuplicate to make it more clear the return value is likely a boolean (by isXXX naming convention) and will be true if it is a duplicate.
3. Remove isMovieDuplicate and use checkIsDuplicate directly in conditional test.
4. Remove the ternary. Assume duplicate failure, only update if not a duplicate and adding to movie array.
5. Use an useEffect hook to persist to localStorage (and eventually to DB).

Code

const addMovie = (movie: IMovie) => {
  let message = 'is already added to your watchlist.';
  let variant = 'warning';

  if (!checkIsDuplicate(movies, movie)) {
    setMovies(movies => sortMovieList([...movies, movie]));
    message = 'has been added to your watchlist.';
    variant = 'success';
  }

  displaySnackbar(`${movie.original_title} ${message}`, { variant });
};

useEffect(() => {
  addMovieToLocalStorage(movies);
}, [movies]);