London | 26-ITP-Jan | Maryanne Mosonik | Sprint 2 | Book Library#467
London | 26-ITP-Jan | Maryanne Mosonik | Sprint 2 | Book Library#467Maryanne-K wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
Maryanne-K
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
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
added
Reviewed
Hi CJ, followed the feedback guideline and made some changes. |
Maryanne-K
added
the
Needs Review
| const titleInput = document.getElementById("title"); | ||
| const authorInput = document.getElementById("author"); | ||
| const pagesInput = document.getElementById("pages"); | ||
| const checkInput = document.getElementById("check"); |
There was a problem hiding this comment.
Normal practice is to declare all shared variables/constants at the beginning of the file, before function definition.
| render(); | ||
| return; | ||
| } | ||
| if (Number.isNaN(pages) || pages <= 0) { |
There was a problem hiding this comment.
Could input be a string representing a decimal number. e.g., "3.1416"?
| showMessage(`Book "${book.title}" deleted!`); | ||
| myLibrary.splice(index, 1); |
There was a problem hiding this comment.
Safer to announce the deletion after the book has been successfully deleted.
cjyuan
added
Complete
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
3 participants
Self checklist
I agree to follow the code of conduct for this organisation.