Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions debugging/book-library/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ function submit() {
alert("Please fill all fields!");
return false;
} else {
let book = new Book(title.value, title.value, pages.value, check.checked);
library.push(book);
//fix: change title.value to author.value
let book = new Book(title.value, author.value, pages.value, check.checked);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nicely done.

//Fix: change library.push(book) to myLibrary.push(book).
myLibrary.push(book);
render();
}
}
Expand All @@ -54,7 +56,9 @@ function render() {
let table = document.getElementById("display");
let rowsNumber = table.rows.length;
//delete old table
for (let n = rowsNumber - 1; n > 0; n-- {
// Fix : add a ) to the end of the for loop condition
// to avoid syntax error.
for (let n = rowsNumber - 1; n > 0; n--) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch of the syntax error here as well.

table.deleteRow(n);
}
//insert updated row and cells
Expand All @@ -76,7 +80,9 @@ function render() {
changeBut.className = "btn btn-success";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst this isn’t a bad variable name, it is good practice to have clear variable names so the purpose can be inferred at a quick glance. For example, changeButton would have been better here. Something to note moving forward.

wasReadCell.appendChild(changeBut);
let readStatus = "";
if (myLibrary[i].check == false) {
// Fix: change myLibrary[i].check to myLibrary[i].check == true
// to check if the book is read or not.
if (myLibrary[i].check == true) {
readStatus = "Yes";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you build applications with status updates, it is good practice to store these statuses in variables/objects. For a small application like this, it is not necessary but as your codebase grows bigger, it becomes easier to slot in the status variable than constantly type in a string (typos could happen). This is something to note.

} else {
readStatus = "No";
Expand All @@ -89,12 +95,15 @@ function render() {
});

//add delete button to every row and render again
// Fix :
// change delBut to delButton because we declare it.
// The event listener is "clicks" instead of "click".
let delButton = document.createElement("button");
delBut.id = i + 5;
deleteCell.appendChild(delBut);
delBut.className = "btn btn-warning";
delBut.innerHTML = "Delete";
delBut.addEventListener("clicks", function () {
delButton.id = i + 5;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See. You used a better variable name here, so try to keep it consistent.

deleteCell.appendChild(delButton);
delButton.className = "btn btn-warning";
delButton.innerHTML = "Delete";
delButton.addEventListener("click", function () {
alert(`You've deleted title: ${myLibrary[i].title}`);
myLibrary.splice(i, 1);
render();
Expand Down