Repository navigation
London | No Cohort | Daniel Wagner-Hall | Sprint 3 | Quote Generator - #1476
illicitestonion wants to merge 2 commits into
Conversation
| document.addEventListener("DOMContentLoaded", () => { showNewQuote(); }); | ||
|
|
||
| document.querySelector("button").addEventListener("click", () => { showNewQuote(); }); |
There was a problem hiding this comment.
On the two event listeners, you’re wrapping showNewQuote in an arrow function that only calls showNewQuote and doesn’t do anything else. Since those arrow functions don’t add any extra logic or clarity, they’re effectively “temporary” functions used once before their immediate use.
In cases like this, you might ask yourself: does this wrapper help readability or behavior in any way, or could I just pass the existing function directly? Here, passing showNewQuote directly would avoid creating an extra, single-use function while keeping the code easy to read.
The destructuring assignment const {quote, author} = pickFromArray(quotes); is a good use of a temporary value: you need a single random choice from quotes and then use both quote and author separately, so holding that result in a variable actually improves clarity and avoids calling pickFromArray twice.
To "like" or "dislike" this comment, please follow this link
|
The CYF AI review has left comments. It will only review a PR one time. When you have addressed these comments, please request review again and a volunteer will take a look. |
Learners, PR Template
Self checklist
Task code
CYF-1096
Changelist
Implements quote generator app.