I’m working on Odin project Library. I have one problem.
When I click the "removeBook" button, it was suppose to remove from the myLibrary array. While it does happen it doesn’t remove the right item from array, and when i add a new book, it sometimes adds two books instead of one.
const addButton = document.querySelector(".add-button");
const bookTitleInput = document.querySelector("#title");
const bookAuthorInput = document.querySelector("#author");
const bookNumOfPagesinput = document.querySelector("#num-of-pages");
const bookReadCheckboxInput = document.querySelector("#read-check");
const form = document.querySelector(".my-form");
const bookDisplay = document.querySelector(".main");
const bookTracker = document.querySelector(".book-tracker");
const pageTracker = document.querySelector(".page-tracker")
const booksReadTracker = document.querySelector(".books-read-tracker")
const myLibrary = [];
function Book(title, author, numOfPages, read) {
this.title = title;
this.author = author;
this.numOfPages = numOfPages;
this.read = read;
}
Book.prototype.pushToArray = function(newBook) {
myLibrary.push(newBook);
}
function addBookToLibary(title, author, numOfPages, read) {
const newBook = new Book(title, author, numOfPages, read);
Book.prototype.pushToArray(newBook);
}
form.addEventListener("submit", function(event) {
event.preventDefault();
const title = bookTitleInput.value;
const author = bookAuthorInput.value;
const numOfPages = bookNumOfPagesinput.value;
let read = "NOT READ";
if (bookReadCheckboxInput.checked) {
read = "READ";
bookReadCheckboxInput.checked = false;
}
addBookToLibary(title, author, numOfPages, read);
console.log(myLibrary);
bookDisplay.textContent = "";
bookTitleInput.value = "";
bookAuthorInput.value = "";
bookNumOfPagesinput.value = "";
myLibrary.forEach((item, index) => {
booksRead = 0;
const div = document.createElement("div");
div.classList.add("book-card");
div.innerHTML = `
<button class="remove-book">X</button>
<p class="title-info">TITLE: ${item.title}</p>
<p class="author-info">AUTHOR: ${item.author}</p>
<p class="pages-info">PAGES: ${item.numOfPages}</p>
`;
const readButton = document.createElement("button");
readButton.classList.add("read-button");
readButton.textContent = item.read;
div.appendChild(readButton);
if (item.read === "READ") {
readButton.style.backgroundColor = "lightgreen";
booksRead += 1;
} else {
readButton.style.backgroundColor = "lightcoral";
};
bookDisplay.appendChild(div);
const removeBook = div.querySelector(".remove-book");
bookTracker.textContent = myLibrary.length;
readButton.addEventListener("click", function() {
if (item.read === "NOT READ") {
readButton.style.backgroundColor = "lightgreen";
item.read = "READ";
booksRead += 1;
} else {
readButton.style.backgroundColor = "lightcoral";
item.read = "NOT READ";
booksRead -= 1;
}
booksReadTracker.textContent = booksRead;
console.log(myLibrary);
})
removeBook.addEventListener("click", function() {
myLibrary.splice(index, 1);
div.remove();
bookTracker.textContent = myLibrary.length;
console.log(myLibrary);
})
})
})<div class="main-container">
<div class="header">
<h1><span><img src="./img/library-outline.svg" alt="libraryIcon"></span>LIBRARY</h1>
</div>
<div class="sidebar">
<div class="info">
<h2>Information</h2>
<p>Total Books in Library: <span class="book-tracker">0</span></p>
<p>Total Books Read: <span class="books-read-tracker">0</span></p>
</div>
<div class="form-container">
<form class="my-form">
<p>
<label for="title">Title</label>
<input type="text" id="title" name="title" required>
</p>
<p>
<label for="author">Author</label required>
<input type="text" id="author" name="author" required>
</p>
<p>
<label for="num-of-pages">Number of Pages</label>
<input type="number" id="num-of-pages" name="num-of-pages">
</p>
<p class="checkbox">
<label for="read-check">Did you read the book?</label>
<input type="checkbox" name="already-read" id="read-check">
</p>
<p>
<button class="add-button" type="submit"><span> + </span> Add Book </button>
</p>
</form>
</div>
</div>
<div class="main">
</div>
<div class="footer">
<p>Made by @Wrncow</p>
</div>
</div>
2
Answers
When you delete an item, the index of all following items changes. But you don’t update the event listener until a new book is submitted.
If you have BookA, BookB, and BookC, and you delete BookB then BookC, the second removal will try to splice an index that doesn’t exist anymore. Unfortunately Javascript doesn’t raise any errors when that happens, and the operation silently fails.
Similarly, if there was a BookD, it’d get deleted instead of BookC.
The div is still removed (because it’s still valid), so the book disappears from the visual list, but it’s still in the array. That’s why you get "extra books" sometimes: the books that failed to be removed get re-added when you submit the form again.
The solution is simple: don’t rely on
indexfrommyLibrary.forEach, but find it each time:You need to delegate the clicks.
Right now you only have ONE event handler on ONE book
You could also be much more DRY. For example I did not use a ternary on the button colour, but I could have.
Note how I add the title to the div as a data attribute. If you want to be able to have two books with the same title, use an ISBN or such as key