Glasgow Class 6 - Mandy Cheung - JS1 - Week 3 - #254
Conversation
I accidentally wrote over this, and forgot to answer it so I've answered it now!
|
great, I like how you're committed to the habit of committing :D |
msimmdev
left a comment
There was a problem hiding this comment.
Well done on completing all the exercises, this is generally to a good standard and your solutions are appropriate.
I noticed a few instances where you're using a for loop with an iterator variable but not declaring your iterator with let or const. This will work, but is bad practice you should make sure to declare all your variables.
I also notices a few instances of using a for loop where a for...of loop would be better, this isn't a big problem, but using for...of for simple iterations makes your code easier to understand and reduces the amount of code you need to write.
| } | ||
| let temperatureReportResults = [ ]; | ||
|
|
||
| for (i = 0; i < cities.length; i++) { |
There was a problem hiding this comment.
Suggestion: Use a for (city of cities) loop here. This reduces the amount of code you need to write and it is easier for a reader to understand your intention.
|
|
||
| let smallestTitle = allArticleTitles[0]; | ||
|
|
||
| for (i = 1; i < allArticleTitles.length; i++) { |
There was a problem hiding this comment.
Issue: The iterator variable i in a for loop should be declared with let. For example
for (let i = 1; i < allArtivlesTitles.length; i++)Suggestion: Use a for...of loop instead of a for loop to improve readability
| let headlinesWithNumbers = [ ]; | ||
| const numbers = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; | ||
|
|
||
| for (i = 0; i < allArticleTitles.length; i++) { |
There was a problem hiding this comment.
Issue: The iterator variable i in a for loop should be declared with let.
Suggestion: Use a for...of loop instead of a for loop to improve readability
| let totalCharactersPerTitle = [ ]; | ||
|
|
||
| // To count the characters per title and add them to totalCharactersPerTitle | ||
| for (i = 0; i < allArticleTitles.length; i++) { |
There was a problem hiding this comment.
Issue: The iterator variable i in a for loop should be declared with let.
Suggestion: Use a for...of loop instead of a for loop to improve readability
|
|
||
| // To store the sum of total characters | ||
| let totalCharacters = 0; | ||
| totalCharactersPerTitle.forEach( item => { |
There was a problem hiding this comment.
Suggestion: This implementation is fine, but this could also be done with a reducer. e.g.
let totalCharaters = totalCharactersPerTitle.reduce((iterator, element) => iterator + element);|
|
||
| let changeInPrice = [ ]; | ||
|
|
||
| for (i = 0; i < array.length; i++) { |
There was a problem hiding this comment.
Issue: The iterator variable i in a for loop should be declared with let.
Suggestion: Use a for...of loop instead of a for loop to improve readability
| // This loop cycles through each stock, the i is the 5 day stock for AAPL, MSFT etc. | ||
| for (i = 0; i < closingPricesForAllStocks.length; i++) { | ||
| let totalStock = 0; | ||
| closingPricesForAllStocks[i].forEach((element) => { |
There was a problem hiding this comment.
Suggestion: Instead of doing * 100 on the calculated average at the end of the function do a * 100 on each element when adding it to totalStock. This lets you do more of the math with integers which reduces the floating point inaccuracies caused by math with floating point numbers.
| let fifthDayPrice = closingPricesArray[i][lengthOfInnerArray - 1]; | ||
| let firstDayPrice = closingPricesArray[i][0]; | ||
|
|
||
| let fifthDayMinusFirstDayRounded = Math.round((fifthDayPrice - firstDayPrice) * 100) / 100; |
There was a problem hiding this comment.
Suggestion do * 100 on fifthDayPrice and firstDayPrice separately so the - operation will occur on integers instead of floating point numbers, reducing floating point math inaccuracy.
| let hello = sayHello(); | ||
| console.log(hello); | ||
|
|
||
| // the function won't return anything, so the variable hello will return undefined. |
There was a problem hiding this comment.
Nitpick: the variable hello will be undefined, try to avoid using the word return for variables and just use it for functions.
|
|
||
| // TODO - Write for loop code here | ||
|
|
||
| for (i = 0; i < WRITERS.length; i++) { |
There was a problem hiding this comment.
Issue: The iterator variable i in a for loop should be declared with let.
No description provided.