Display search results for TV shows

Question

I haven't been coding super long in this language so if someone could take a look and help me improve my code and make it more efficient, that would be great! The HTML is one input field that you can type a show and then you get the result on the screen. From the beginning you get the top ten shows with the highest rating from the API.

// from start show the tv shows with highest rating
fetch('https://api.tvmaze.com/shows')
    .then(blob => blob.json())
    .then(json => {
        const topTenShows = json
        .filter(show => show.rating.average) 
        .sort((a, b) => a.rating.average > b.rating.average ? -1 : 1) 
        .slice(0, 9) // tar element 0-9 i arrayen
         return topTenShows
  })
  .then(shows => {
      const app = document.getElementById('app')

      app.innerHTML = shows.map(show => `
          <div class="col-sm movie-content">
              <div class="movie-image">
                  ${show.image ? `<img src="${show.image.medium}">` : `<img class="fallbackImage"src="design/icons/No_image_available.svg">`}
              </div>
              <div class="movie-info">
                  <h1>${show.name}</h1>
              </div>
          </div>
      `).join(' ');
    })

// show the search results from user input
function searchTvShows ({ target }) {
    fetch(`https://api.tvmaze.com/search/shows?q=${target.value}`)
        .then(blob => blob.json())
        .then(shows => {
            const app = document.getElementById('app');

            app.innerHTML = shows.map(({ show }) => `
                <div class="col-sm movie-content">
                    <div class="movie-image">
                        ${show.image ? `<img src="${show.image.medium}">` : `<img class="fallbackImage"src="design/icons/No_image_available.svg">`}
                    </div>
                    <div class="movie-info">
                        <h1>${show.name}</h1>
                    </div>
              </div>

        `).join(' ');
    })
  }

  const inputSearchField = document.querySelector('.inputSearchField')

  inputSearchField.addEventListener('keydown', searchTvShows)

Answer 1

• Since in both instances HTML is the same, it could be extracted to another function;
• target.value appended to the URL needs to go through EncodeURIComponent() to avoid bugs, surprises and even XSS;
• topTenShows has actually only 9 shows. Second argument of .slice() is exclusive;
• It would be better to listen to input event instead of keydown. Right now, when you type, the last character doesn't make it into request.

Rewrite

const app              = document.querySelector('#app'),
      inputSearchField = document.querySelector('.inputSearchField'),
      apiEndpoint      = 'https://api.tvmaze.com',
      noImagePath      = 'design/icons/No_image_available.svg';

/**
 * For each object representing one show, creates HTML source and sets them all as content of #app
 * @param  {object} shows Array containing objects representing API results
 * @return {string}       HTML that got set as content of #app (unused)
 */
const buildShowsHTML = shows => {
  app.innerHTML = shows.map(obj => {
    const show = obj.show || obj;
    const img = show.image ? `<img src="${show.image.medium}"/>` : `<img class="fallbackImage" src="${noImagePath}"/>`;

    return `
      <div class="col-sm movie-content">
        <div class="movie-image">
          ${img}
        </div>
        <div class="movie-info">
          <h1>${show.name}</h1>
        </div>
      </div>`;
  }).join('');
};

/**
 * Gets top 10 shows from the input array of objects
 * @param  {object} arr Array containing objects representing API results
 * @return {object}     Array of top 10 shows
 */
const getTopTenShows = arr => arr
  .filter(show => show.rating.average)
  .sort((a, b) => a.rating.average > b.rating.average ? -1 : 1)
  .slice(0, 10); // tar element [0 – 10) i arrayen

// Show top 10 shows upon load
fetch(`${apiEndpoint}/shows`).then(resp => resp.json()).then(getTopTenShows).then(buildShowsHTML);

// Show the search results on user input
inputSearchField.addEventListener('input', event => {
  const input = encodeURIComponent(event.target.value);
  fetch(`${apiEndpoint}/search/shows?q=${input}`).then(resp => resp.json()).then(buildShowsHTML);
});

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8"/>
  <meta name="viewport" content="width=device-width"/>
  <link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css" rel="stylesheet" type="text/css"/>
  <title>Demo</title>
</head>
<body>
  <input class="inputSearchField"/>
  <main id="app"></main>
</body>
</html>

Answer 2

• Both blocks of code effectively do the same thing during fetch and render. The only differentiator is the query, which determines how the url is constructed and how it's post-processed. Use a conditional to conditionally run this logic instead of repeating the code.

• I'd be cautious about sticking arbitrary data into HTML this way. This is very prone to XSS as well as break your layout unexpectedly. Make sure the data you stick in there is properly formatted/encoded.

• If the API supports query parameters that filter the results, I recommend using that instead of filtering on the client side. That way, you don't have to pull in data that you don't actually render.

• Since you're using fetch, there's a high probability your target platforms also support async functions. Consider using that for a simpler, less then-y async code.

• While it's ok to build URLs by manually interpolating strings, this assumes that you're encoding the values correctly. Use URL and URLSearchParams to construct urls instead.

• Modern runtimes, as far as I know, will warn you when you have unhandled promises (promises with no catch somewhere). Be sure to add one.

Answer 3

Thank you for posting back your question. As promised here is my answer.

Some comments

Here are a few points I should mention:

• I like your code because you're using quite a lot of Array's methods and template literals and you're handling the asynchronous call nicely.

• the first time I read the code I immediately noticed there were two identical chunks of code: mainly the HTML formatting part, it is redundant and can use a good refactoring. This way you can easily reduce the number of repeated pieces of code. Also, there are too many levels in your function searchTvShows.

• you may want to choose an alternative to the classic .innerHTML +=. Adding HTML to the page with this method forces the browser to re-render the DOM which is not very efficient. You could instead create a new HTML element and append it to an existing one.

• there are hardly any comments in your code (you should add more meaningful comments to help understand your code. For example this statement .slice(0, 9) doesn't need to have the comment // tar element 0-9 i arrayen because it's obvious.

• you're using both .querySelector and .getElementById, maybe you should just stick with the latter, it's more efficient to use because you can select by CSS selector. Also why some HTML elements have id others have a class? You can minimize the number of times you call these methods by preparing an object containing all the used elements. This way you can access them without needing to reselect them.

• You may want to use the keyup event instead of the keydown. The latter will run the function for the before last letter. So it is not updating the search properly.

• there is no consistency in the use of semicolons ;.

---

Refactored code

const elements = {
  input: document.querySelector('#inputSearchField'),
  app: document.querySelector('#app')
};

// filters applied on the recieved data objects
const topTen = json => json
   .filter(show => show.rating.average)
   .sort((a, b) => a.rating.average > b.rating.average ? -1 : 1)
   .slice(0, 9)

const fixMap = shows => shows.map(s => s.show)

// fetch and show search results from API source
const searchTvShows = (filter = false, target = undefined) => {
  const url = `https://api.tvmaze.com/${target ? `search/shows?q=${target}` : `shows`}`;
  console.log(url)
  fetch(url)
    .then(blob => blob.json())
    .then(json => filter ? filter(json) : json)
    .then(shows => {
      elements.app.innerHTML = shows.map((show) => formatShow(show)).join(' ') || "<div>No match</div>";
    });
}

// HTML formatting of a show
const formatShow = (show) => `
    <div class="col-sm movie-content">
      <div class="movie-image">
          ${show.image
              ? `<img src="${show.image.medium}">`
              : `<img class="fallbackImage" src="design/icons/No_image_available.svg">`
          }
      </div>
      <div class="movie-info">
          <h1>${show.name}</h1>
      </div>
    </div>
    `

This way you can call the featured list and the result list with the same function searchTvShows:

1. featured films:

   searchTvShows(topTen)
                   ↑
             filter applied
             to the search
   

2. updating search results:

   elements.input.addEventListener('keyup',()=>searchTvShows(fixMap, event.target.value))
              ↑                                                 ↑            
         access saved                                        filter to   
         HTML element                                    fix the response
                                                        ({show}) vs (show)
   

const elements = {
  input: document.querySelector('#inputSearchField'),
  app: document.querySelector('#app')
};

// filters applied on the recieved data objects
const topTen = json => json
   .filter(show => show.rating.average)
   .sort((a, b) => a.rating.average > b.rating.average ? -1 : 1)
   .slice(0, 9)

const fixMap = shows => shows.map(s => s.show)

// fetch and show search results from API source
const searchTvShows = (filter = false, target = undefined) => {
  const url = `https://api.tvmaze.com/${target ? `search/shows?q=${target}` : `shows`}`;
  console.log(url)
  fetch(url)
    .then(blob => blob.json())
    .then(json => filter ? filter(json) : json)
    .then(shows => {
      elements.app.innerHTML = shows.map((show) => formatShow(show)).join(' ') || "<div>No match</div>";
    });
}

// HTML formatting of a show
const formatShow = (show) => `
    <div class="col-sm movie-content">
      <div class="movie-image">
          ${show.image
              ? `<img src="${show.image.medium}">`
              : `<img class="fallbackImage" src="design/icons/No_image_available.svg">`
          }
      </div>
      <div class="movie-info">
          <h1>${show.name}</h1>
      </div>
    </div>
    `

elements.input.addEventListener('keyup', () => searchTvShows(fixMap, event.target.value))
searchTvShows(topTen)

.fallbackImage {
  background: lightgrey;
  height: 200px;
  width: 200px;
}

<input id="inputSearchField">
<div id="app"></div>