refactor: apply CJ second round feedback

Author: mahmoudshaabo1984

debugging/book-library/script.js

@@ -1,66 +1,83 @@
-// myLibrary is the main array — like a shelf that holds all book objects
-let myLibrary = [];
+// const is safer than let here because myLibrary is never reassigned —
+// we only call .push() and .splice() on it, which modify its contents, not the variable itself
+const myLibrary = [];
 
-// Use const for DOM references because these elements never change
-// "const" means: this variable will always point to the same element
+// Declare all shared DOM references at the top of the file
+// These are const because they always point to the same HTML elements
 const titleInput = document.getElementById("title");
 const authorInput = document.getElementById("author");
 const pagesInput = document.getElementById("pages");
 const readCheckbox = document.getElementById("check");
 
 // Wait for the page to fully load before running any code
-// addEventListener is safer than window.onload because it allows
-// multiple listeners to be registered without overwriting each other
 window.addEventListener("load", function () {
   addDefaultBooks();
   render();
 });
 
 // Add two default books so the page is not empty on first load
-// Renamed from "populateStorage" — that name implied localStorage usage
 function addDefaultBooks() {
   if (myLibrary.length === 0) {
-    myLibrary.push(new Book("Robinson Crusoe", "Daniel Defoe", "252", true));
+    // Page count is stored as a NUMBER (252, not "252") — correct data type
+    myLibrary.push(new Book("Robinson Crusoe", "Daniel Defoe", 252, true));
     myLibrary.push(
-      new Book("The Old Man and the Sea", "Ernest Hemingway", "127", true)
+      new Book("The Old Man and the Sea", "Ernest Hemingway", 127, true)
     );
   }
 }
 
 // Book is a constructor function — a template for creating book objects
-// Think of it as a blank contact card: every book fills in the same four fields
 function Book(title, author, pages, hasBeenRead) {
   this.title = title;
   this.author = author;
   this.pages = pages;
-  // Renamed "check" to "hasBeenRead" — much clearer what this field means
   this.hasBeenRead = hasBeenRead;
 }
 
+// sanitiseText trims leading and trailing spaces from a string
+// Prevents accepting inputs that contain only spaces as valid values
+function sanitiseText(value) {
+  return value.trim();
+}
+
+// sanitisePages converts the pages input to a safe integer
+// Rejects scientific notation like "3e2" which would display strangely
+function sanitisePages(value) {
+  const trimmed = value.trim();
+  // If the input contains "e" or "E", it is scientific notation — reject it
+  if (trimmed.toLowerCase().includes("e")) {
+    return NaN;
+  }
+  const parsed = Number(trimmed);
+  if (trimmed === "" || isNaN(parsed) || parsed <= 0) {
+    return NaN;
+  }
+  return Math.floor(parsed);
+}
+
 // Called when the user clicks Submit
-// Renamed from "submit" to avoid conflict with the browser's built-in submit behaviour
 function addBook() {
-  // Validate that all fields are filled before adding the book
-  if (
-    titleInput.value === "" ||
-    authorInput.value === "" ||
-    pagesInput.value === ""
-  ) {
-    alert("Please fill all fields!");
+  // Step 1: Sanitise the inputs — clean them before checking or using them
+  const cleanTitle = sanitiseText(titleInput.value);
+  const cleanAuthor = sanitiseText(authorInput.value);
+  const cleanPages = sanitisePages(pagesInput.value);
+
+  // Step 2: Validate — reject empty strings and invalid page numbers
+  if (cleanTitle === "" || cleanAuthor === "" || isNaN(cleanPages)) {
+    alert("Please fill all fields with valid values!");
     return;
   }
 
-  // Create a new Book object from the form values
+  // Step 3: Create and store the new book using the cleaned values
   const newBook = new Book(
-    titleInput.value,
-    authorInput.value,
-    pagesInput.value,
+    cleanTitle,
+    cleanAuthor,
+    cleanPages,
     readCheckbox.checked
   );
-
   myLibrary.push(newBook);
 
-  // Clear the form fields after adding the book
+  // Step 4: Clear the form fields after adding the book
   titleInput.value = "";
   authorInput.value = "";
   pagesInput.value = "";
@@ -70,18 +87,12 @@ function addBook() {
 }
 
 // createReadButton builds and returns a toggle button for a single book
-// Extracting this into its own function makes render() easier to read
 function createReadButton(index) {
   const button = document.createElement("button");
   button.className = "btn btn-success";
-
-  // Use textContent instead of innerHTML for safety
-  // textContent never runs any HTML or scripts — innerHTML can be dangerous
-  // if the text comes from user input
   button.textContent = myLibrary[index].hasBeenRead ? "Yes" : "No";
 
   button.addEventListener("click", function () {
-    // Flip the read status: true becomes false, false becomes true
     myLibrary[index].hasBeenRead = !myLibrary[index].hasBeenRead;
     render();
   });
@@ -96,7 +107,8 @@ function createDeleteButton(index) {
   button.textContent = "Delete";
 
   button.addEventListener("click", function () {
-    // splice(index, 1) removes exactly 1 item at position "index"
+    // Show an alert with the deleted book's title — restoring original behaviour
+    alert(`"${myLibrary[index].title}" has been deleted.`);
     myLibrary.splice(index, 1);
     render();
   });
@@ -107,29 +119,23 @@ function createDeleteButton(index) {
 // render() redraws the entire table from the current myLibrary array
 function render() {
   const table = document.getElementById("display");
-  const rowCount = table.rows.length;
 
-  // Delete all rows except the header row (row 0)
-  for (let i = rowCount - 1; i > 0; i--) {
-    table.deleteRow(i);
-  }
+  // Clear the tbody in one step — simpler and faster than deleting rows one by one
+  const tbody = table.querySelector("tbody");
+  tbody.innerHTML = "";
 
-  // Build one row per book using helper functions
   for (let i = 0; i < myLibrary.length; i++) {
-    const row = table.insertRow();
+    const row = tbody.insertRow();
     const titleCell = row.insertCell(0);
     const authorCell = row.insertCell(1);
     const pagesCell = row.insertCell(2);
     const readCell = row.insertCell(3);
     const deleteCell = row.insertCell(4);
 
-    // Use textContent for user data — never innerHTML — to prevent XSS attacks
-    // XSS (Cross-Site Scripting) is when a malicious script gets injected into a page
     titleCell.textContent = myLibrary[i].title;
     authorCell.textContent = myLibrary[i].author;
     pagesCell.textContent = myLibrary[i].pages;
 
-    // Append the buttons created by the helper functions
     readCell.appendChild(createReadButton(i));
     deleteCell.appendChild(createDeleteButton(i));
   }