London | 26-ITP-May | Raihan Sharif | Sprint 2 | Book Library#497
London | 26-ITP-May | Raihan Sharif | Sprint 2 | Book Library#497RaihanSharif wants to merge 26 commits into
Conversation
Duplicate: title and author both the same as an existing book. Not case sensitive.
innerHTML can be a security risk when dealing with user input
simpler change button classlist logic user confirm instead of alert in delete button replace all unnecessary let to const
RaihanSharif
added
📅 Sprint 2
charset and the name attributes cannot be in the same meta tag
illicitonion
added
Review in progress
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
LGTM, well done! Just one question to consider
| class="form-control" | ||
| id="author" | ||
| name="author" | ||
| minlength="2" |
There was a problem hiding this comment.
How did you decide on these minimum and maximum lengths (for all fields)?
There was a problem hiding this comment.
Tbh, I didn't give it a lot of thought wrt the lengths.
I put 2 as min with the rationale that accidental typos and author name conflicts would be less likely, but that's not a strong argument.
I had considered a min of 1, as there are single letter pseudonyms and book titles.
The max I set to something very long as size isn't a bottleneck at this scale, but the user still has some sort of limit.
There was a problem hiding this comment.
After some consideration, I have replaced the min with 1, and the max with 255 as that is usually the limit for database VARCHAR.
There was a problem hiding this comment.
Seems reasonable :) I'm less convinced on the max page limit (there are some very long books in this worked), but you have reasonable thoughts on them :)
There was a problem hiding this comment.
Had to double check my code to see if I accidentally put 255 limit on pages! I'll leave it at 10,000.
Thank you for your feedback. I have learned to be more thoughful and deliberate with my code.
illicitonion
added
Reviewed
RaihanSharif
added
Needs Review
illicitonion
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Books can be added or deleted
Books can be marked as completed or not at the time of adding them to list
Books can be marked as completed or not after adding
Code is checked and updated for quality against the feedback guideline here: https://github.com/CodeYourFuture/Module-Data-Flows/blob/general-review-feedback/debugging/book-library/feedback.md