London-ITP-Jan2025| Sabita Shrestha| Module-Data-Flows| Book-library| week2

[SabitaShrestha325]

Learners, PR Template

Self checklist

• I have committed my files one by one, on purpose, and for a reason
• I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
• I have tested my changes
• My changes follow the style guide
• My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

[cjyuan]

I think there are some improvements you can still make to the JS code.

Also, when I checked the code in index.html at https://validator.w3.org/, it indicated some errors.

[cjyuan]

Instead of deleting the table rows one by one, can you think of a more efficient way to remove all rows (except the <th>...</th>) in the table?

[cjyuan]

In terms of input validation,

1. Are all input properly checked?
2. Can .value be null? (Do we need to check someInputElement.value == null?)
3. What if a user enters only space characters in the "title" input field?
4. What if a users enters -1 or 3.1416 in the "pages" input field?

With the way the book's title is assigned to an HTML element (line 69), a book with a title containing special character sequence such as <i> can possibly ruin the display.

Lines 75, 89:

• Is the value assigned to these id attribute unique?
• Is there a need to assign an id attribute to delButton?
• Is there a need to assign an id attribute to changeBut (at line 72)?

Lines 74, 88:

• Can you think of a more consistent way to name the variables representing the two buttons, changeBut and delButton?

[SabitaShrestha325]

Hi, thank you for your review. I have made the changes as mentioned; could you please review my work and let me know if anything needs to be adjusted? Thank you.

[cjyuan]

On separate note, the self checklist and the Changelist in the PR template is not filled.

May I suggest learning the "importance to fill the PR template in a Github PR" via Google search or ChatGPT?

[cjyuan]

Changes to the JS code look great! Wonderful job. (Can consider also fix the error in HTML when you have time). Feel free to mark this as "Complete" anytime.