London 9 - Lovelace - Shayan Mahnam - JS-Core-1 - Week 3#164
London 9 - Lovelace - Shayan Mahnam - JS-Core-1 - Week 3#164ShayanMahnam wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
jonnywyatt
left a comment
jonnywyatt
left a comment
There was a problem hiding this comment.
Brilliant, some advanced techniques here
It's important that code can be easily read by other developers, so don't do too much in a single line, break it into steps. It's better if the code is slightly longer but more readable.
Well done!
2-mandatory/3-financial-times.js
Outdated
| */ | ||
| function potentialHeadlines(allArticleTitles) { | ||
| // TODO | ||
| return allArticleTitles.filter(items => items.length < 65); |
There was a problem hiding this comment.
The instruction asks for 65 characters or less
2-mandatory/3-financial-times.js
Outdated
| for (let i = 0; i < allArticleTitles.length; i++) { | ||
| arr.push(allArticleTitles[i].split(" ").length); | ||
| } | ||
| return allArticleTitles[arr.indexOf(Math.min(...arr))]; |
There was a problem hiding this comment.
It can be more readable to break it into more lines. eg
const fewestWords = Math.min(...arr)
const indexOfArticle = arr.indexOf(fewestWords)
const articleTitle = allArticleTitles[indexOfArticle]
return articleTitle
Also, this works when all articles have a different number of words. What if two or more articles have the same number of words? arr.indexOf will find the first match.
There was a problem hiding this comment.
looks better now thank you 🥇
3-extra/1-factorial.js
Outdated
| function factorial(input) { | ||
| // TODO | ||
| let sum = 1 | ||
| for (i = 1; i <= input; i++){ |
There was a problem hiding this comment.
Don't forget to initialise i with let
2 participants
No description provided.