3

I'm building a React App which consumes an API and what I build for now is 2 functions which both GET the same URL but changing the second part of the API Endpoint like URL/?search or URL/?i=123 but what I would like to achieve is to have less redundant code so I was wondering to make one function which just takes the same URL and change the second part of the URL based on the call.

By the way, this approach gives me problems.

The code I did originally was this:

import {API_MOVIE_URL, API_KEY} from "./ApisConst";

export const getMoviesBySearch = async search => {
  try {
    const url = `${API_MOVIE_URL}?apikey=${API_KEY}&${search}`;
    const response = await fetch(url);
    const json = await response.json();
    return json;
  } catch {
    return {
      success: false,
      result: [],
      message: "There is an issue to get data from server. Please try again later.",
    };
  }
};

export const getMoviesInfo = async movieID => {
  try {
    const url = `${API_MOVIE_URL}?apikey=${API_KEY}&i=${movieID}&plot`;
    const response = await fetch(url);
    const json = await response.json();
    return json;
  } catch {
    return {
      success: false,
      result: [],
      message: "There is an issue to get data from server. Please try again later.",
    };
  }
};     

And the change I tried is:

const fetchAPI = async ({type, query}) => {
  const queryParams = {
    byString: `&${query}`,
    byMovieId: `&i=${query}&plot`,
  };

  const endpoint = `${API_MOVIE_URL}?apikey=${API_KEY}${queryParams[type]}`;
  console.log("fetching", endpoint);

  return fetch(endpoint)
    .then(res => res)
    .catch(() => ({
      success: false,
      result: [],
      message: "There is an issue to get data from server. Please try again later.",
    }));
};

export const getMoviesBySearch = async search =>
  await fetchAPI({type: "byString", query: search});

export const getMoviesInfo = async movieID =>
  await fetchAPI({type: "byMovieId", query: movieID});

But this second approach gives me an error in the console which is:

Response {type: "cors", url: "https://www.omdbapi.com/?apikey=API_KEY&s=harry+potter&type=movie", redirected: true, status: 200, ok: true, …}
body: (...)
bodyUsed: false
headers: Headers {}
ok: true
redirected: true
status: 200
statusText: ""
type: "cors"
url: "https://www.omdbapi.com/?apikey=API_KEY&s=harry+potter&type=movie"

The first approach works perfectly but the second one none and trying to get a solution but cannot really think what to do to get this code better optimized.

Jakub
  • 2,367
  • 6
  • 31
  • 82
  • As far as I can tell, the only issue with your code was that you accidentally wrote `.then(res => res)` instead of `.then(res => res.json())` What you saw in the console isn't an error but the Response object. The one you then call `.json()` on. –  Nov 26 '19 at 23:17
  • Here's how to turn an object into a query string: https://stackoverflow.com/a/1714899/5734311. You can easily expand this for you `fetchAPI` function. –  Nov 26 '19 at 23:26
  • Here's my approach: https://jsfiddle.net/khrismuc/gut3bfxj/ –  Nov 26 '19 at 23:43

1 Answers1

4

Since the queries are identical except for the url, create a query generator (createMoviesQuery) that accepts a function that generates the url (urlGenerator), and returns a query function

Example (not tested):

import {API_MOVIE_URL, API_KEY} from "./ApisConst";

const createMoviesQuery = urlGenerator => async (...params) => {
  try {
    const url = urlGenerator(...params);
    const response = await fetch(url);
    const json = await response.json();
    return json;
  } catch {
    return {
      success: false,
      result: [],
      message: "There is an issue to get data from server. Please try again later.",
    };
  }
};

export const getMoviesBySearch = createMoviesQuery((search) => `${API_MOVIE_URL}?apikey=${API_KEY}&${search}`);

export const getMoviesInfo = createMoviesQuery((movieID) => `${API_MOVIE_URL}?apikey=${API_KEY}&i=${movieID}&plot`);
Ori Drori
  • 183,571
  • 29
  • 224
  • 209
  • 1
    That worked perfectly :) but I'm learning also so could you explain easily what was the issue with mine and what is your approach with urlGenerator so I can understand for the next time – Jakub Nov 26 '19 at 23:16
  • I'm downvoting this because it's just a rewrite of OP's code that doesn't mention what the error with the original code is, and the two functions at the bottom are actually worse then before imo. –  Nov 26 '19 at 23:20
  • @ChrisG I do not understand much of what you commented but if you feel this solution is bad could you maybe propose one you feel is better then as I would like to find a solution to my problem using the best way if possible. – Jakub Nov 26 '19 at 23:22
  • He's not complaining about my code. He's complaining about the lack of reason why your code didn't work. – Ori Drori Nov 26 '19 at 23:24
  • Your code seems fine, except for the `.then(res => res)` that should be `.then(res => res.json)`, and the response in the console is 200 (ok). – Ori Drori Nov 26 '19 at 23:26
  • Why I prefer passing a function that will generate a url, then creating the URL inside the code? After stripping out the url building code, you can see that you've got a generic API function. You can use it to generate query functions for the movie api, but you can also use it for other apis. – Ori Drori Nov 26 '19 at 23:28
  • Ok, I see however, I'm sorry if I wrote not clear my issue, by the way, there were no errors with the original code as I specified but the issue was with the new code and that I asked as was not finding a way to resolve and I showed the error asking a help as I was not sure why of that. – Jakub Nov 26 '19 at 23:30
  • That's fine. I've taken your question more of "how can I", then "fix my code". – Ori Drori Nov 26 '19 at 23:31