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>
1 Answer 1
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
}