Skip to content
Open
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
94 changes: 62 additions & 32 deletions debugging/book-library/script.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,42 @@
// Book class
class Book {
constructor(title, author, pages, check) {
const validated = constructorValidation(title, author, pages, check);

this.title = validated.title;
this.author = validated.author;
this.pages = validated.pages;
this.check = validated.check;
}
}

// Checking if constructor arguments are valid
function constructorValidation(title, author, pages, check) {
if (typeof title !== "string" || title.trim() === "") {
throw new Error("Title must be a non-empty string");
}

if (typeof author !== "string" || author.trim() === "") {
throw new Error("Author must be a non-empty string");
}

const pagesNum = Number(pages);
if (isNaN(pagesNum) || pagesNum <= 0) {
throw new Error("Pages must be a positive number");
}

if (typeof check !== "boolean") {
throw new Error("Check must be a boolean");
}

return {
title: title.trim(),
author: author.trim(),
pages: pagesNum,
check,
};
}

let myLibrary = [];

window.addEventListener("load", function (e) {
Expand All @@ -6,17 +45,16 @@ window.addEventListener("load", function (e) {
});

function populateStorage() {
if (myLibrary.length == 0) {
let book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true);
let book2 = new Book(
if (myLibrary.length === 0) {
const book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true);
const book2 = new Book(
"The Old Man and the Sea",
"Ernest Hemingway",
"127",
true
);
myLibrary.push(book1);
myLibrary.push(book2);
render();
}
}

Expand All @@ -28,33 +66,25 @@ 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 == ""
) {
if (title.value === "" || author.value === "" || pages.value === "") {
Copy link

Choose a reason for hiding this comment

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

Good validation check 👍
However, if you do not remove unnecessary whitespaces from the start and the end, " " will pass through this check as it's a bunch of valid characters and is not an empty string. How do you think you could counter this?

alert("Please fill all fields!");
return false;
} else {
let book = new Book(title.value, title.value, pages.value, check.checked);
library.push(book);
let book = new Book(title.value, author.value, pages.value, check.checked);
myLibrary.push(book);
render();
title.value = "";
author.value = "";
pages.value = "";
check.checked = false;
}
}

function Book(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);
}
//insert updated row and cells
Expand All @@ -76,11 +106,7 @@ function render() {
changeBut.className = "btn btn-success";
wasReadCell.appendChild(changeBut);
let readStatus = "";
if (myLibrary[i].check == false) {
readStatus = "Yes";
} else {
readStatus = "No";
}
myLibrary[i].check ? (readStatus = "Yes") : (readStatus = "No");
changeBut.innerText = readStatus;
Copy link

Choose a reason for hiding this comment

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

Have you come across Ternary Operator? Could make the code cleaner:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Conditional_operator


changeBut.addEventListener("click", function () {
Expand All @@ -90,13 +116,17 @@ 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 () {
alert(`You've deleted title: ${myLibrary[i].title}`);
myLibrary.splice(i, 1);
//delButton.id = i + 5;
delButton.id = `delButton_${i}`;
deleteCell.appendChild(delButton);
delButton.className = "btn btn-warning";
delButton.textContent = "Delete";
delButton.dataset.index = i;
delButton.addEventListener("click", function (e) {
Copy link

Choose a reason for hiding this comment

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

a better way to do this would be using data- attributes. You're using i inside a closure which, while fine, is not the best practice.
jQuery has excellent APIs for using data attributes. Even with core JavaScript, if you can master data attributes, you will no longer need to do these, and can use HTML itself for storing that reference.
https://developer.mozilla.org/en-US/docs/Web/HTML/How_to/Use_data_attributes

const index = e.target.dataset.index;

alert(`You've deleted title: ${myLibrary[index].title}`);
myLibrary.splice(index, 1);
render();
});
}
Expand Down
Loading