Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

London Class 8 - Matilda Ako - JS Core 1 Coursework - Week 3 - #24

Open
MatildaAko wants to merge 8 commits into
CodeYourFuture:mainfrom
MatildaAko:main
Open

London Class 8 - Matilda Ako - JS Core 1 Coursework - Week 3#24
MatildaAko wants to merge 8 commits into
CodeYourFuture:mainfrom
MatildaAko:main

Conversation

@MatildaAko

Copy link
Copy Markdown

No description provided.

@MatildaAko

Copy link
Copy Markdown
Author

@haack My work is ready for review

@haack haack left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice 🚀 I've commented a few extension thoughts on how to make a couple of solutions slightly more concise or to explore another way to solve them.

for (title of allArticleTitles) {
titles.push(title.length);
}
let total = titles.reduce((a, b) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I'd recommend avoiding very short variable names in a reduce because it can make it hard to know what they refer to.

Also does reduce make sense to you? Can you think of other situations where reduce is helpful?

let total = titles.reduce((a, b) => {
return a + b;
})
return parseInt(total / titles.length)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need parseInt here?

function averageNumberOfCharacters(allArticleTitles) {
// TODO
let titles = [];
for (title of allArticleTitles) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want a little extension you can try writing this function without the for loop step. Tip: have a think about how you can do it with just the reduce function below!

Comment thread 2-mandatory/4-stocks.js
let closingPrices = closingPricesForAllStocks;
let averagePrices = [];
for (let prices of closingPrices) {
let total = prices.reduce((a, b) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above about short variable names

Comment thread 2-mandatory/4-stocks.js
(Apple's price on the 5th day) - (Apple's price on the 1st day) = 172.99 - 179.19 = -6.2
The price change value should be rounded to 2 decimal places, and should be a number (not a string)
*/
function rounded(num) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

Comment thread 2-mandatory/4-stocks.js
// TODO
}
let closingPrices = closingPricesForAllStocks;
let stock = stocks;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

Comment thread 2-mandatory/4-stocks.js
let stock = stocks;
let highestArr = [];

for (let i = 0; i < closingPrices.length; i++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Can you think of another way how you could find the highest without a for loop? (tip: what if you could easily get the highest element and lowest element)

Comment thread 3-extra/1-factorial.js
}
let result = num.reduce((a, b) => {
return a * b;
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a more concise way to do this. (Tip: how could you do this without the reduce and only using the while loop above?)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants