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

London-10-Elena-Barker_ JavaScript-Core-1-Coursework-Week3 - #251

Open
ElenaBarker wants to merge 3 commits into
CodeYourFuture:mainfrom
ElenaBarker:main
Open

London-10-Elena-Barker_ JavaScript-Core-1-Coursework-Week3#251
ElenaBarker wants to merge 3 commits into
CodeYourFuture:mainfrom
ElenaBarker:main

Conversation

@ElenaBarker

Copy link
Copy Markdown

weather report and financial times done

weather report and financial times done
output.push (string);

}
return output;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment: Well done!
Consider: You could also use iterate variable as below.
function getTemperatureReport(cities) {
const temperatureReport = [];
for(let i = 0; i < cities.length; i++) {
const temperature = temperatureService(cities[i]);
temperatureReport.push(The temperature in ${cities[i]} is ${temperature} degrees);
}
return temperatureReport;
}

allTitles.push(headline)
}
}
return allTitles;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment: Same solution as mine


return shortestLine;

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment: I see you use 100000; as a value. I assume it is a made up value.

Consider: Have a look at how you can use [i] It is not used in any of your coding.

Example:
function titleWithFewestWords(allArticleTitles) {
let shortestTitle = allArticleTitles[0];

for (let i = 1; i < allArticleTitles.length; i++) {
    let currentTitle = allArticleTitles[i];
    if (currentTitle.length < shortestTitle.length) {
        shortestTitle = currentTitle  
    }
}
return shortestTitle;

}

}
}
return arrayWithNumbers;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment: I see you use parseInt. Nice. Different solution to mine

totalCharacters = totalCharacters + headline.length;
}
return Math.round(totalCharacters / allArticleTitles.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.

Comment: Nice!

task finished with all tests passed
@@ -12,7 +12,15 @@
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good 👍
One suggestion - it's a good idea to use let or const when declaring variables. This code will still work without it, but when you start working on larger programs - you could run into problems.

@@ -5,7 +5,13 @@
Implement the function below, which will return a new array containing only article titles which will fit.
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Almost perfect - but same comment as above about using let or const 😄
For an extra challenge, try re-writing the below function using the filter array method.

Implement the function below to return a new array containing all the headlines which contain a number.
(Hint: remember that you can also loop through the characters of a string if you need to)
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice solution!
I like that you've used a separate function to check if the string has a number. It makes the code very easy to read 👍

Comment thread 2-mandatory/3-stocks.js
@@ -34,7 +34,18 @@ const CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_STOCKS = [
Functions can help with this!
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great job!
Now that you have something that works - maybe you could split some of the code into a separate function improve readability. For example, you could have a function which just calculates the average price given the stockPricesLastFiveDays array.

@moneyinthesky

Copy link
Copy Markdown
Contributor

Great work on this coursework @ElenaBarker! Those stocks exercises are tough - you had some really nice solutions 😄

@moneyinthesky moneyinthesky added the reviewed A mentor has reviewed this code label Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

reviewed A mentor has reviewed this code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants