West-Midlands | 26-ITP-May | Maryam Janjua | Sprint 2 | Book Library - #573
West-Midlands | 26-ITP-May | Maryam Janjua | Sprint 2 | Book Library#573maryam-devio wants to merge 1 commit into
Conversation
maryam-devio
added
🏕 Priority Mandatory
Khantdotcom
added
Review in progress
Khantdotcom
left a comment
Khantdotcom
left a comment
There was a problem hiding this comment.
Mandatory changes required:
- populateStorage()
|
|
||
| window.addEventListener("load", function (e) { | ||
| populateStorage(); | ||
| render(); |
There was a problem hiding this comment.
Can you check your populateStorage() function whether it is single purpose function? Hint : you're doing something twice. I assume this is a typo but read this article to better understand Single Responsibility Principle here.
| @@ -31,15 +31,21 @@ | |||
| if ( | |||
| title.value == null || | |||
There was a problem hiding this comment.
If the input is empty, it returns an empty string "", it will never return null.
| title.value == "" || | ||
| author.value == null || | ||
| author.value == "" || | ||
| pages.value == null || | ||
| pages.value == "" |
There was a problem hiding this comment.
You can use the .trim() method for checking and javascript's truthiness to improve the logic. Read this article -> here
| myLibrary.splice(i, 1); | ||
| render(); | ||
| }); | ||
| delButton.id = i + 5; |
There was a problem hiding this comment.
Instead of using the id attribute, modern JavaScript uses data- attributes to store hidden data (like the array index) on elements. Just a tip , not mandatory changes needed. Read this article here.
2 participants
Learners, PR Template
Self checklist
Changelist
In this task, we were required to identify and debug errors in the Book Library application. I fixed the bugs related to displaying, adding, and deleting books, as well as the author and read status. I also tested the application to make sure the required functionality works correctly.