London|26-ITP-January|Alexandru Pocovnicu|Sprint 3| quote generator app#984
London|26-ITP-January|Alexandru Pocovnicu|Sprint 3| quote generator app#984alexandru-pocovnicu wants to merge 13 commits intoCodeYourFuture:mainfrom
Conversation
…ent listeners for better readability
| // call pickFromArray with the quotes array to check you get a random quote | ||
|
|
||
| function chooseQuote() { | ||
| const randomQuote = quotes[Math.floor(Math.random() * quotes.length)]; |
There was a problem hiding this comment.
welldone you have done a good job just a few things:
can we make the line easy to understand research KISS and SOC and DRY. can we do something to avoid repeating the line 496? its used in line 20 too
| let interval = null; | ||
| autoGenerate.addEventListener("change", () => { | ||
| if (autoGenerate.checked) { | ||
| interval = setInterval(chooseQuote, 2000); |
There was a problem hiding this comment.
what doeas 2000 mean? what are magic numbers ? and how can we handle magic numbers ?
| const randomQuote = quotes[Math.floor(Math.random() * quotes.length)]; | ||
|
|
||
| const quote = document.getElementById("quote"); | ||
| quote.innerText = randomQuote.quote; |
There was a problem hiding this comment.
can we use textContent instead of innerText and why ?
what happens if quote is null ?
|
thank you |
| const randomQuote = pickFromArray(quotes) | ||
|
|
||
| const quote = document.getElementById("quote"); | ||
| if(quote!==null){ |
There was a problem hiding this comment.
please check spacing here and elsewhere, use the style guide as reference
| interval = setInterval(chooseQuote, changeQuoteInterval); | ||
| } else { | ||
| clearInterval(interval); | ||
| interval = null; |
There was a problem hiding this comment.
this line may not be necessary, try removing it and see if it works?
There was a problem hiding this comment.
which line? the "if" starts the interval and without the "else" the interval doesn't stop even after the box is unchecked
There was a problem hiding this comment.
the line that this comment is on, line 521 which says interval = null;
|
thank you |
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
Learners, PR Template
Self checklist
Changelist
wrote code to show random quote and author