North-West-5/Shimen-Afshar/JavaScript-Core-1-Coursework-Week3 - #145
North-West-5/Shimen-Afshar/JavaScript-Core-1-Coursework-Week3#145ShimenAfshar wants to merge 3 commits into
Conversation
Ekremteke
left a comment
There was a problem hiding this comment.
Hi Shima these are so impressive. I could not find anything to add and to look your PR was instructive for me.
| // TODO | ||
| let report=[] | ||
| for(let i = 0; i < cities.length; i++){ | ||
| report[i]=`The temperature in ${cities[i]} is ${temperatureService(cities[i])} degrees`; |
There was a problem hiding this comment.
nice use of template literals! here you can use push, for example report.push(The temperature in ${cities[i]} is ${temperatureService(cities[i])} degrees)
| do{ | ||
| counter=generateRandomNumber(); | ||
| } | ||
| while(counter<50); |
There was a problem hiding this comment.
nice do-while loop implementation :) remember to use <= here rather than just < as the returned number needs to be greater than 50
| function potentialHeadlines(allArticleTitles) { | ||
| // TODO | ||
| if(allArticleTitles.lengh !== 0){ | ||
| allArticleTitles = ARTICLE_TITLES.filter((el) => el.length <= 65); |
There was a problem hiding this comment.
here you are using a mixture of allArticleTitles which is passed into the function and ARTICLE_TITLES (just use allArticlesTitles). Good use of the filter array method, but you don't need the if statement, for example if you do let shortTitles = allArticleTitles.filter((el) => el.length <= 65) and then return this, it will return an empty array if allArticleTitles.length is 0 :)
| function titleWithFewestWords(allArticleTitles) { | ||
| // TODO | ||
| let arr = allArticleTitles; | ||
| let newArr = arr.sort((a, b) => a.length - b.length); |
There was a problem hiding this comment.
good use of sort here, to sort an array without mutating the original array you can use the spread operator: let newArr = [...allArticleTitles].sort((a, b) => a.length - b.length);. We could use a different array name here to make the code more readable, for example changing newArr to something like fewestWordsArr
| } | ||
| } | ||
| return arr; | ||
|
|
There was a problem hiding this comment.
regex is really useful for functions like this - this is a good site to learn more & have a play around https://regex101.com/
for the above you can do something like (/\d/.test(stringYouWantToTest)) the \d checks for any digit, returning true if a digit is found
| for (let articleTitle of allArticleTitles) { | ||
| total += articleTitle.length; | ||
| } | ||
| average = total / allArticleTitles.length; |
There was a problem hiding this comment.
really nice solution well done, you could just define average on line 63, for example let average = total / allArticleTitles.length; and get rid of your definition on line 59 (as it's not really doing anything at the moment)
| for (let i = 0; i < CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_STOCKS.length; i++) { | ||
| for (let j = 0; j < CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_STOCKS[i].length; j++) { |
There was a problem hiding this comment.
good solution, well done on using multiple loops!
| "The highest price of " + stocks[i].toUpperCase() + " " + "in the last 5 days was " + highestNum.toFixed(2)); | ||
|
|
||
| } | ||
| return highestPrice; |
There was a problem hiding this comment.
nice solution :) The Math.max method is useful for finding the highest number, you can pass in an array like so:
let highestNum = Math.max(...closingPricesForAllStocks[i])
No description provided.