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

WM4 - class 1 - Simeon Bikov - JS - week 3 - #108

Open
simeonbikov wants to merge 15 commits into
CodeYourFuture:mainfrom
simeonbikov:main
Open

WM4 - class 1 - Simeon Bikov - JS - week 3#108
simeonbikov wants to merge 15 commits into
CodeYourFuture:mainfrom
simeonbikov:main

Conversation

@simeonbikov

Copy link
Copy Markdown

No description provided.

Comment thread 2-mandatory/1-weather-report.js Outdated

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you think you're missing anything on this line?

*/
function potentialHeadlines(allArticleTitles) {
// TODO
let shortArticles = allArticleTitles.filter(title => title.length < 66);

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 usage of .filter() 👍

Comment thread 2-mandatory/3-financial-times.js Outdated
// TODO
let arrOfLength = [];
for (let i = 0; i < allArticleTitles.length; i++) {
arrOfLength.push(allArticleTitles[i].split(" ").length - 1);

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 did you use .push() here instead of arrOfLength[i]?

Naming variables is hard. What data is being stored in the arrOfLength variable? Can you think of a better name?

Comment thread 2-mandatory/3-financial-times.js Outdated
*/
function headlinesWithNumbers(allArticleTitles) {
// TODO
articlesWithNumbers = [];

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 is missing from this line of code? Why does it still work without the missing piece of code?

Comment thread 2-mandatory/3-financial-times.js Outdated
*/
function averageNumberOfCharacters(allArticleTitles) {
// TODO
let sum = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Naming is hard. What is this the sum of? Can you think of a better variable name?

Comment thread 2-mandatory/4-stocks.js
function getPriceChanges(closingPricesForAllStocks) {
// TODO
let changeInPrice = [];
let firstPrice = 0;

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 is the benefit of defining these variables outside of the for loop?

Comment thread 2-mandatory/4-stocks.js
let highestPrice = 0;

for (let i = 0; i < closingPricesForAllStocks.length; i++) {
highestPrice = Math.max(...closingPricesForAllStocks[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.

Nice use of the spread operator 👍

Comment thread 2-mandatory/4-stocks.js
function highestPriceDescriptions(closingPricesForAllStocks, stocks) {
// TODO
let highestPriceForAllStock = [];
let highestPrice = 0;

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 the benefit of defining this variable outside of the for loop?

@jdbevan jdbevan 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.

Nice work Simeon! You've solved some hard problems here :)

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