London | 26-ITP-Jan | Angela McLeary | Sprint 2 | Book Library by AngelaMcLeary · Pull Request #415 · CodeYourFuture/Module-Data-Flows

AngelaMcLeary · 2026-04-09T19:17:07Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

Debugging of book library app and fixing typos, syntaxes and validation to mention a few.

…80 and 82

cjyuan

Can you check if any of this general feedback can help you further improve your code?
https://github.com/CodeYourFuture/Module-Data-Flows/blob/general-review-feedback/debugging/book-library/feedback.md

Doing so can help me speed up the review process. Thanks.

cjyuan · 2026-04-12T21:37:13Z

debugging/book-library/script.js

 function populateStorage() {
  if (myLibrary.length == 0) {
-    let book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true);
+    let book1 = new Book("Robinson Crusoe", "Daniel Defoe", "252", true);


Did you notice the third parameter is a string and not a number?

Hi @cjyuan, Thanks for your feedback. I did not notice that, thanks for pointing it out. Since the input is a string, I have converted the pages to numbers.

cjyuan · 2026-04-12T21:38:28Z

debugging/book-library/script.js

  if (
    title.value == null ||
    title.value == "" ||
+    author.value == null ||


Why compare .value to null? When will .value ever be null?

Hi @cjyuan, Thank you for your feedback. You are right, .value is never null. It returns a string or "" when empty. So that check was unnecessary. I have also refactored that code block and removed the unnecessary checks.

cjyuan · 2026-04-12T21:46:15Z

debugging/book-library/script.js

  } 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);


Using raw input value without properly pre-processing them is dangerous.

Here are some input that can "mess up" the app:

title.value = " "; page.value = "3e-2";

cjyuan · 2026-04-12T21:54:17Z

debugging/book-library/script.js

@@ -1 +1 @@
 let myLibrary = [];


Your latest commit changed const back to let.

It's a good practice to click the "File changes" in your own PR to review (double check) the code your submitted.

cjyuan · 2026-04-12T21:55:45Z

debugging/book-library/script.js

    let readStatus = "";
    if (myLibrary[i].check == false) {
-      readStatus = "Yes";
-    } else {
      readStatus = "No";
+    } else {
+      readStatus = "Yes";
    }
-    changeBut.innerText = readStatus;
+    changeBtn.innerText = readStatus;


This could be a good opportunity to practice using the ? : conditional operator. Can you rewrite the code on lines 88–94 as a single statement?

Hi @cjyuan, Thank you for your feedback. Good suggestion — I’ve replaced the if statement with a ternary operator for a cleaner single-line expression.

cjyuan · 2026-04-12T21:56:34Z

debugging/book-library/script.js

-    delBut.innerHTML = "Delete";
-    delBut.addEventListener("clicks", function () {
+    let deleteBtn = document.createElement("button");
+    deleteBtn.id = i + 5;


Lines 85, 103:

Are the values assigned to these id attributes unique?

Is there any need to assign an id attribute to either buttons?

Hi @cjyuan, Thank you for your feedback. Good point - the id attributes were based on the loop index, so they are not guaranteed to be unique after deletions. Also since I'm not using them for DOM selection, so they are unnecessary, so I've removed both (changeBtn.id and delete.id).

cjyuan · 2026-04-12T21:57:59Z

debugging/book-library/script.js

+    deleteCell.appendChild(deleteBtn);
+    deleteBtn.className = "btn btn-warning";
+    deleteBtn.innerHTML = "Delete";
+    deleteBtn.addEventListener("click", function (index) {


Why introduce the parameter index to the callback function?

Hi @cjyuan, Thank you for your feedback. That is definitely a mistake on my part. Furthermore the callback doesn't receive a custom index, only the event object. I wasn't even using it, I've deleted it. I have also refactored the code.

cjyuan · 2026-04-12T21:58:59Z

debugging/book-library/script.js

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


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.

…hecks

AngelaMcLeary added 9 commits April 9, 2026 19:08

missing bracket closure on line 57

6fe2b67

fix syntax error on line 92 update variable

b647506

add title to html file

dc2969c

update variable name to myLibrary on Line 41

6146e7f

return of author value on line 40 to allow correct output

731ac4c

fix typo change click to clicks on line 92

377bd24

fix result of input status of book when read is selected to YES line …

200d98c

…80 and 82

update validation requiremtns on lines 36-41

6d570e5

fix formatting issues in submit and render functions

ff6b347

AngelaMcLeary added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 9, 2026

cjyuan reviewed Apr 10, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 10, 2026

AngelaMcLeary added 16 commits April 10, 2026 12:30

update input type for title and author

376af2f

update improper meta tag

1d6d91b

update html tag to include language

366d541

update remove empty rows

957ce0b

update scripts to include defer

b6bb86f

update add form element for user data input

c87d77d

update to remove onclick submit from html

89af4d0

update to fix remove form and onclick

1a97cd5

update to remove invalid css in form-group

6822947

update code to clear fields after submission

5350f6e

update main container in html

9228c2f

update css to meet accessibility requirements

5062215

update spacing for form

10ae61f

update css for h1, h2 and update changeBut and deleteBut to ..Btn

ece0823

update let to const for myLibrary variable

6b1fb53

update variable delbut to deleteBtn

380c723

AngelaMcLeary added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 12, 2026

cjyuan reviewed Apr 12, 2026

View reviewed changes

cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 12, 2026

AngelaMcLeary added 5 commits April 13, 2026 12:06

update converted page number from string to number

820a4e1

update submit function to handle page as numbers and fix validation c…

f170475

…hecks

update if staement and use tenary operator

94388ba

update unneccessay code changeBtn.id and deleteBtn.id

eb6981c

update remove index parameter from deleteBtn.addEventListener

adfb8c0

Uh oh!

Conversation

AngelaMcLeary commented Apr 9, 2026

Learners, PR Template

Changelist

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants