Skip to content
Closed
Changes from 3 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
74 changes: 47 additions & 27 deletions debugging/book-library/script.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
let myLibrary = [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we declare myLibrary in a way that prevents it from being accidentally reassigned?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you address this comment?


window.addEventListener("load", function (e) {
window.addEventListener("load", function() {
populateStorage();
render();
});
Expand Down Expand Up @@ -28,33 +28,53 @@ const check = document.getElementById("check");
//check the right input from forms and if its ok -> add the new book (object in array)
//via Book function and start render function
function submit() {
if (
title.value == null ||
title.value == "" ||
pages.value == null ||
pages.value == ""
) {
alert("Please fill all fields!");

// 1. Preprocess and store cleaned values in variables / removes spaces from the beginning and end
const cleanTitle = title.value.trim();
const cleanAuthor = author.value.trim();

// Convert string input to a Number for calculation/validation
const pageCount = Number(pages.value);
const isRead = check.checked;

// 2. VALIDATION: // Check if strings are empty after trimming spaces
if (cleanTitle === "" || cleanAuthor === "") {
alert("Title and Author cannot be empty!");
return false;
}
// Check if page count is a valid number and greater than 0
if (isNaN(pageCount) || pageCount <= 0) {
alert("Please enter a valid number of pages!");
return false;
} else {
let book = new Book(title.value, title.value, pages.value, check.checked);
library.push(book);
}

// 3. If all validations pass, create a new Book object and add it to the library
let book = new Book(cleanTitle, cleanAuthor, pageCount, isRead);
myLibrary.push(book);

// 4. Clear the form fields after submission
title.value = "";
author.value = "";
pages.value = "";
check.checked = false;

render();
}
}

function Book(title, author, pages, check) {
this.title = title;
this.author = author;
this.pages = pages;
this.check = check;
class Book {
constructor(title, author, pages, check) {
this.title = title;
this.author = author;
this.pages = pages;
this.check = check;
}
}

function render() {
let table = document.getElementById("display");
let rowsNumber = table.rows.length;
//delete old table
for (let n = rowsNumber - 1; n > 0; n-- {
for (let n = rowsNumber - 1; n > 0; n--) {
table.deleteRow(n);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is easier and more efficient just to clear <tbody>; no need to use loop.

//insert updated row and cells
Expand All @@ -66,17 +86,17 @@ function render() {
let pagesCell = row.insertCell(2);
let wasReadCell = row.insertCell(3);
let deleteCell = row.insertCell(4);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These variables are also not going to be reassigned.

titleCell.innerHTML = myLibrary[i].title;
authorCell.innerHTML = myLibrary[i].author;
pagesCell.innerHTML = myLibrary[i].pages;
titleCell.textContent = myLibrary[i].title;
authorCell.textContent = myLibrary[i].author;
pagesCell.textContent = myLibrary[i].pages;

//add and wait for action for read/unread button
let changeBut = document.createElement("button");
changeBut.id = i;
changeBut.className = "btn btn-success";
wasReadCell.appendChild(changeBut);
let readStatus = "";
if (myLibrary[i].check == false) {
if (myLibrary[i].check) {
readStatus = "Yes";
} else {
readStatus = "No";
Expand All @@ -90,11 +110,11 @@ function render() {

//add delete button to every row and render again
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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Lines 95, 113:
    • id attribute should be unique. Are the values assigned to these id attributes unique?
    • Is there any need to assign an id attribute to either buttons?

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();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The alert message is shown before the book is actually deleted; the deletion only occurs after the alert dialog is dismissed. This introduces a risk that the operation may not complete (e.g., if the user closes the browser before dismissing the alert).

In general, it’s better to display a confirmation message only after the associated operation has successfully completed.

Expand Down
Loading