London | 26-ITP-May | Martin Mwaka | Sprint 2 | Book library

[Temceo]

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

Debug and improve functionality of the library app

[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.

[Temceo]

I have reviewed the general feedback and improved the code

[cjyuan]

To make locating all the "code that is to be executed once on page load" easier, a common practice is to keep the code in the same place. For example, inside the page's onload callback function.

[Temceo]

Hi, can I please clarify on this point. The addForm event listener only runs each time a new book is added and the processEntries() function validates the inputs before adding the new book. The code that is run on page load is the populateStorage() function which calls the render() function to show all existing books in the library

[Temceo]

I have updated the event listeners for window load and add book so hopefully now it is much clearer

[cjyuan]

1. Basically all statements written in the top-level scope are code that would be executed once when the HTML page first loaded.

• Code that attach a callback to a DOM element once usually falls into this category.

2. Typically we order the code in a file the following manner:

  variable declarations
  
  function definitions
  
  code to be executed (code to be executed once is placed here)

3. Alternatively, we can place all the code to be executed once on page load inside a function (e.g., init()), and then just call the function.

---

What you did works.

The code that manages form validation could be better organised but that's out of the scope of this exercise.

[cjyuan]

1. Basically all statements written in the top-level scope are code that would be executed once when the HTML page first loaded.

• Code that attach a callback to a DOM element once usually falls into this category.

2. Typically we order the code in a file the following manner:

  variable declarations
  
  function definitions
  
  code to be executed (code to be executed once is placed here)

3. Alternatively, we can place all the code to be executed once on page load inside a function (e.g., init()), and then just call the function.

---

What you did works.

The code that manages form validation could be better organised but that's out of the scope of this exercise.

[cjyuan]

Why not use the name addBookForm? It makes it clearer that the variable refers to a form element.

[Temceo]

I have reverted this to addBookForm

[cjyuan]

Changes look good.

[cjyuan]

Should the .pages property of a book be a number or a string? For this particular simple app it may not matter but it is safer to consistently represent a kind of data using same data type.