3
\$\begingroup\$

In my app I have 3 components:

  1. MovieSearchComponent to "house" the other 2 components
  2. MovieSearch to search an API and retrieve the data
  3. MovieResultList to display the data in the browser

Both components are wrapped in a parent container MovieSearchComponent.

My MovieSearchComponent looks like this:

interface Movie {
  original_title: string;
  id: string;
}

const MovieSearchComponent = () => {
  const [movieList, setMovies] = useState<Movie[]>([]);

  const addMovie = ((movies: Movie[]) => {
    setMovies([...movies]);
  });

  return (
    <React.Fragment>
      <MovieSearch addMovie={addMovie} />
      <MovieResultList movieList={movieList}/>      
    </React.Fragment>
  )
}

In here I have a empty MovieList array that uses the setMovies function to fill the MovieList array. There's also a addMovie function that gets called from the MovieSearch component and takes a array as a parameter. Then I pass the MovieList array to the MovieResultList component.

The MovieSearch component:

const Search = styled.input`
  color: green;
`

const MovieSearch = ( {addMovie }) => {  
    const apikey = 'api_key=***************dad4';
    const baseurl = 'https://api.themoviedb.org/3/search/movie?'

    const searchTMDBapi = (e) => {
        e.persist()
        setMovieSearchResults(e);
    }

    const setMovieSearchResults = debounce((e) => {
        const query = e.target.value;
        fetch(baseurl + apikey + '&language=en-US&query=' + query + '&page=1&include_adult=false')
          .then(response => response.json())
          .then(data => addMovie(data.results))
  }, 500);

  return <Search placeholder="Search" onChange={searchTMDBapi}/>
}

In this function component I render a input field by using styled components. The input field calls the searchTMDBapi function when something is typed. In that method I call the setMovieSearchResults method which calls the api and sets the api data in the Hook by using .then(data => addMovie(data.results))

The addMovie Hook updates the movieList array in MovieSearchComponent and the <MovieResultList movieList={movieList}/> syntax passes the movieList array to the MovieResultList which renders it:

const MovieResultList = (props) => {
    return (
        <div>
            <ul>
                {props.movieList.map(movie => {
                    return (<li key={movie.id}>{movie.original_title}</li>)
                })}
            </ul>
        </div>
    )
}

My goal was to create 2 components. 1 to retrieve data from a external API and the other to display that data in a different component. I succeeded in that but I was wondering if there are some aspects on which I could improve this code and my React knowledge.

\$\endgroup\$
1
\$\begingroup\$

MovieSearchComponent

  • With the useState the common pattern is to name the state update function matching with the state value, i.e. movieList and setMovieList. This makes it clear what state the function will update.
  • I had to read and reread your description of addMovie and compare it against the actual implementation. It isn't clear if the function is to add a movie, add an array of movies, or, based on the implementation, replace existing movie array list. The name of the function and the result of the function executing should match. In your description you say addMovie updates the movielist array, so for the sake of code review I'll assume that to mean the third option with state replacement. In this case the callback function can simply be setMovieList. Update the prop name as well.

Code suggestions

import React, { Fragment } from 'react';

...

const MovieSearchComponent = () => {
  const [movieList, setMovieList] = useState<Movie[]>([]);

  return (
    <Fragment>
      <MovieSearch setMovieList={setMovieList} />
      <MovieResultList movieList={movieList}/>      
    </Fragment>
  )
};

MovieSearch

  • Update prop addMovie to setMovieList.
  • Values like apiKey and baseurl that never change don't need to be in the functional component body and continually redefined. Declare them externally in the file.
  • It may be better to extract the synthetic event data and allow the event object to be returned to the event pool as quickly as possible. Extract and pass the event's target's value to setMovieSearchResults.
  • Handle the fetch request's unhappy paths, i.e. a not-successful responses or errors handling successful responses. Checking that the fetch was successful.
  • You should always standardize and sanitize user input.

Code suggestions

const apikey = 'api_key=***************dad4';
const baseurl = 'https://api.themoviedb.org/3/search/movie?'

const Search = styled.input`
  color: green;
`;

const MovieSearch = ({ setMovieList }) => { 
    const searchTMDBapi = e => {
        const { value } = e.target;

        // Simple example: URI encode and lowercase
        // ... there's more you could do
        const query = encodeURI(value.toLowerCase());

        setMovieSearchResults(query);
    }

    const setMovieSearchResults = debounce(query => {
        fetch(baseurl + apikey + '&language=en-US&query=' + query + '&page=1&include_adult=false')
          .then(response => {
            if (!response.ok) {
              throw new Error('Network response was not ok');
            }
            return response.json();
          })
          .then(data => setMovieList(data.results))
          .catch(console.error);
  }, 500);

  return <Search placeholder="Search" onChange={searchTMDBapi}/>
}

MovieResultList

  • Pretty simple & clean component. Only suggestion here might be to tighten up variables by using object destructuring.

Code suggestions

const MovieResultList = ({ movieList }) => (
  <div>
    <ul>
      {movieList.map(({ id, original_title }) => <li key={id}>{original_title}</li>)}
    </ul>
  </div>
);
\$\endgroup\$
2
  • \$\begingroup\$ Thanks a lot for the comments. Naming "things" in code is harder than actually writing code! \$\endgroup\$ – Peter Boomsma Aug 2 '20 at 13:04
  • \$\begingroup\$ @PeterBoomsma "One of the most difficult problems in Computer Science" :) Welcome. \$\endgroup\$ – Drew Reese Aug 2 '20 at 16:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.