Glasgow | May-2025 | Mansoor Munawar | Sprint 2 | Book library by MansoorM11 · Pull Request #320 · CodeYourFuture/Module-Data-Flows

MansoorM11 · 2025-08-20T21:06:02Z

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Briefly explain your PR.
fixed the bugs in book library and refactor code to meet the requirements of the task. And also used "https://github.com/cjyuan/Module-Data-Flows/blob/book-library-feedback/debugging/book-library/feedback.md" to improve my code

Questions

Ask any questions you have for your reviewer.

…r settings

cjyuan

Most of the the suggestions in the feedback document I shared with you were not taken into consideration. Please go through the document thoroughly.

cjyuan · 2025-08-20T21:47:00Z

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("Robison Crusoe", "Daniel Defoe", 252, true);


Why change one and not the other?

i totally missed that. Thanks for letting me know

cjyuan · 2025-08-20T21:51:16Z

debugging/book-library/script.js

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


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

…riable names

…rator

cjyuan

According to https://validator.w3.org/, there are errors in your index.html. Can you fix these errors?

cjyuan · 2025-08-21T06:22:47Z

debugging/book-library/script.js

+    !titleInput.value.trim() ||
+    !authorInput.value.trim() ||
+    !pagesInput.value.trim()
  ) {
    alert("Please fill all fields!");
    return false;
  } else {
-    let book = new Book(title.value, title.value, pages.value, check.checked);
-    library.push(book);
+    myLibrary.push(
+      new Book(
+        titleInput.value.trim(),
+        authorInput.value.trim(),
+        Number(pagesInput.value),
+        checkInput.checked
+      )
+    );


For better performance (reduce number of function calls) and reducing the chance of using raw input accidently, we could stored the pre-processed/sanitized/normalized input in some variables first, and reference the variables in other part of the function.

pagesInput.value could be a value like "3.1416".

cjyuan · 2025-08-21T06:26:07Z

debugging/book-library/script.js

+    let deleteBtn = document.createElement("button");
+    deleteBtn.id = i + 5;


Variable name is still not consistent

Value assigned to the id attribute is not unique (when there are more than 5 books), and what is the use of the id attribute in this application?

…idation

…plify event listener setup

cjyuan

Changes look good. Well done.

MansoorM11 added 3 commits August 20, 2025 21:44

Fix book submission logic and improve render function; add live serve…

7373cdb

…r settings

Remove unnecessary render call in populateStorage function

3af9173

Fix book page count type in populateStorage function

e288ff5

MansoorM11 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2025

cjyuan reviewed Aug 20, 2025

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 Aug 20, 2025

MansoorM11 added 5 commits August 20, 2025 22:56

Fix book pages value type in populateStorage function

39444d7

Refactor submit function to use ternary statment.

dcd06c6

Rename input variable constants for clarity

584176d

Refactor submit function to improve readability and use consistent va…

3809f61

…riable names

Refactor read status assignment in render function to use ternary ope…

d883baa

…rator

MansoorM11 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 20, 2025

cjyuan reviewed Aug 21, 2025

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 Aug 21, 2025

MansoorM11 added 3 commits August 21, 2025 10:27

Fix HTML structure and input types for title and author fields

a1b43bd

Refactor submit function for better performance and improve input val…

2b070dc

…idation

Refactor render function to improve button naming consistency and sim…

e84f17c

…plify event listener setup

MansoorM11 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 21, 2025

cjyuan reviewed Aug 21, 2025

View reviewed changes

cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 21, 2025

MansoorM11 mentioned this pull request Aug 21, 2025

Glasgow | Data Flows | Mansoor Munawar MansoorM11/My-Coursework-Planner#70

Open

illicitonion closed this Sep 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Glasgow | May-2025 | Mansoor Munawar | Sprint 2 | Book library#320

Glasgow | May-2025 | Mansoor Munawar | Sprint 2 | Book library#320
MansoorM11 wants to merge 11 commits intoCodeYourFuture:mainfrom
MansoorM11:book-library-1

MansoorM11 commented Aug 20, 2025

Uh oh!

cjyuan left a comment

Uh oh!

cjyuan Aug 20, 2025

Uh oh!

MansoorM11 Aug 20, 2025

Uh oh!

cjyuan Aug 20, 2025

Uh oh!

cjyuan left a comment

Uh oh!

cjyuan Aug 21, 2025

Uh oh!

cjyuan Aug 21, 2025

Uh oh!

cjyuan left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		let deleteBtn = document.createElement("button");
		deleteBtn.id = i + 5;

Uh oh!

Conversation

MansoorM11 commented Aug 20, 2025

Questions

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

MansoorM11 Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants