3
\$\begingroup\$

The code will create a new Movie class and then with the response of the API call, will render html using the data.

Although the code works, I imagine it will get quite convoluted, now that everything after the new class is created, has to be nested into the event listener. There must be a more correct way to do this. For example, would it be possible to get the response to a new Class object without creating a new class instance inside of the API call?

/*separate function for api call. It gets the movie data*/
function getMovie(movieTitle) {
  return fetch(`https://www.omdbapi.com/?t=${movieTitle}&apikey=3861f60e`)
    .then((res) => res.json())
    .then((data) => {
      console.log(data);
      return data;
    });
}


/*Movie Class*/
class Movie {
constructor(data){
  Object.assign(this, data);
  }
  
  renderMovie() {
    return ` 
    <div class="movie-container">
    <div class="image-container">
           <img src='${this.Poster}'/>
    </div>
    <div class="movie-content-container">
      <div class="title">
        <h4>${this.Title}</h4>
        <p>${this.imdbRating}</p>
      </div>
      <div class="movie-details">
        <p>${this.Runtime}</p>
        <p>${this.Genre}</p>
        <button class="add-watchlist-btn">Watchlist</button>
      </div>
      <div class="movie-desc">
        <p>${this.Plot}</p>
      </div>
    </div>
    </div>
  `;
  }
}

const container = document.querySelector(".container");
const searchPage = document.querySelector(".search-page-container");
const searchBtn = document.querySelector(".search-btn");
const searchInput = document.querySelector(".input-search-bar");


/* If I create the instance inside of here, then anything that I need to do further, has to be nested inside. This doesn't feel right at all*/
searchBtn.addEventListener("click", (e) => {
  e.preventDefault();
  getMovie(searchInput.value).then((res) => {
    container.removeChild(searchPage);
    container.innerHTML += new Movie(res).renderMovie();
    let btn = document.querySelector(".add-watchlist-btn");
    btn.addEventListener("click", () => {
      console.log("button clicked");
    });
  });
  searchInput.value = "";
});
<!DOCTYPE html>
<html>
  <head>
    <title>Movie App</title>
    <meta charset="UTF-8" />
    <link rel="stylesheet" href="src/styles.css">
    <script src="https://kit.fontawesome.com/facd5daff4.js" crossorigin="anonymous"></script>
  </head>

  <body>
    <div id="app">
      <div class="container">
        <header>
          <div class="header-group">
              <h1>Find Your Film</h1>
              <h4><a>My Watchlist</a></h4>
          </div>
                </header>
                <form>
        <div class="search-bar">
          <label for="search">
            <input
              id="search"
              class="input-search-bar"
              type="text"
              name="search"
              placeholder="Search for a movie"
            />
          </label>
            <button class="search-btn">Search</button>
                    </form>
         </div>
         <div class="search-page-container">
           
         </div>
       </div>
     </div>

    <script src="src/index.js"></script>
  </body>
</html>

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

I see a few problems:

  • you create a class instance with object.assign from a JSON call, which is quite risky. You don't always know what properties the json will return.
  • your class returns html, but then later you add a click function to that html outside of the class, which looks very un-OOP to me. A class should be responsible for all its own content.
  • It seems you need the class only to generate HTML. In that case it might work better as a function.
  • you create HTML as text strings which is very error prone.

I would try to organise the code something like this.

searchBtn.addEventListener("click", async(e) => {
  e.preventDefault()
  let res = await fetch(`https://www.omdbapi.com/?t=${searchInput.value}&apikey=3861f60e`)
  let json = await res.json()
  // TODO CHECK HERE IF JSON ACTUALLY HAS MOVIE DATA
  container.removeChild(searchPage)
  searchInput.value = ""
  let div = renderMovie(json)
  container.appendChild(div)
});

// Just an example
function renderMovie(json) {
   let moviediv = document.createElement("div")
   moviediv.classList.add("movie-container")
   let title = document.createElement("h4")
   title.innerText = json.title
   let button = document.createElement("button")
   button.innerText = "add to watchlist"
   button.addEventListener("click", () => {
      console.log("added to watchlist")
   })
   moviediv.appendChild(title)
   moviediv.appendChild(button)
   return moviediv
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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