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

London 10 Maksim Lukianenko JavaScript-Core-1-Coursework-Week3 - #218

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

London 10 Maksim Lukianenko JavaScript-Core-1-Coursework-Week3#218
maxbmaapc wants to merge 8 commits into
CodeYourFuture:mainfrom
maxbmaapc:main

Conversation

@maxbmaapc

Copy link
Copy Markdown

No description provided.


function getTemperatureReport(cities) {
// TODO
const temperatureReport = [];

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 work 👍

*/
function potentialHeadlines(allArticleTitles) {
// TODO
const headTitle = allArticleTitles.filter(title => title.length < 65);

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 is a nice implementation!
I would ask you double-check one thing: the comment above says "65 characters or less". Could you update this code to meet that requirement?

*/
function titleWithFewestWords(allArticleTitles) {
// TODO
let shortestTitle = allArticleTitles[0];

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 great 😄

*/
function averageNumberOfCharacters(allArticleTitles) {
// TODO
const articleSum = allArticleTitles.reduce((acc, val) => acc + val.length, 0);

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.

Nice use of reduce here!

Comment thread 2-mandatory/3-stocks.js
*/
function getAveragePrices(closingPricesForAllStocks) {
// TODO
let currentStock = [];

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 😄
A suggestion: you might be able to improve the readability of this code by creating a separate function which calculates the average for a single stock. Each function will be a little bit simpler, and the new function will have a name - so we can more easily see what it's doing.

Comment thread 2-mandatory/3-stocks.js

for (let i = 0; i < closingPricesForAllStocks.length; i++) {
const numberOfPrices = closingPricesForAllStocks[i].length - 1;
const firstDay = closingPricesForAllStocks[i].at(0);

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.

It's also possible here to use bracket notation twice. For example, closingPricesForAllStocks[i][0].

Comment thread 2-mandatory/3-stocks.js
for (let i = 0; i < closingPricesForAllStocks.length; i++) {
const stockName = stocks[i].toUpperCase();
const stockPrices = closingPricesForAllStocks[i];
const highestPrice = Math.max(...stockPrices).toFixed(2);

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.

Nice use of Math.max here 👍

@moneyinthesky

Copy link
Copy Markdown
Contributor

Overall, this is very good work - great job!

@moneyinthesky moneyinthesky added the reviewed A mentor has reviewed this code label Mar 14, 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.

2 participants