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

Glasgow 6 - Hussein Al-Sayed - JS1 week3 - #237

Open
hussein-alsayed wants to merge 14 commits into
CodeYourFuture:mainfrom
hussein-alsayed:main
Open

Glasgow 6 - Hussein Al-Sayed - JS1 week3#237
hussein-alsayed wants to merge 14 commits into
CodeYourFuture:mainfrom
hussein-alsayed:main

Conversation

@hussein-alsayed

@hussein-alsayed hussein-alsayed commented Mar 6, 2023

Copy link
Copy Markdown

No description provided.

from 1st try
B-)
not solved copletly, but one attempt closer to success. and couple chatgpt fix suggustions, wich neither works but closer to success.
stocks array elements linked to their coresponding price array seccussfully.
little issue however, last string's highest price orints only with onw decimal place!
Comment thread 2-mandatory/1-weather-report.js Outdated

citiesReport.push(statement);
}
return citiesReport;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

make sure to keep indentation to the proper level across the code block

Comment thread 2-mandatory/2-financial-times.js Outdated
*/
function titleWithFewestWords(allArticleTitles) {
// TODO
let shortestTitle = allArticleTitles[1];

@sztupy sztupy Mar 6, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't forget that arrays are indexed from 0, not 1. If your list would only contain a single article this code would fail

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

right, yeah.. hard to get the new values (0=1) into my brain xD

Comment thread 2-mandatory/2-financial-times.js
Comment thread 2-mandatory/2-financial-times.js Outdated

for (const title of allArticleTitles){
for (const part of title){
for (const character of part){

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 much much better way to calculate the length of a string. In fact you alredy used it in the first exercise in this file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

right, looks much better now.

Comment thread 2-mandatory/3-stocks.js
// TODO
let averagePrices = [];

for (const stock of CLOSING_PRICES_LAST_5_DAYS_FOR_ALL_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.

indentation is off in multiple places in this loop. Keeping them properly aligned makes it easier to see the flow of the code. Your IDE can help you with that, most also contain a hotkey to auto-indent and fix other styling issues

@sztupy

sztupy commented Mar 6, 2023

Copy link
Copy Markdown

Nice solutions utilizing what you have learned in Week 3. Note the couple of suggestions, and the bug that was raised.

@hussein-alsayed

Copy link
Copy Markdown
Author

@sztupy many thanks for the valuable notes. I've updated the codes accordingly.
I remember you also commented on my indentation on a previous code, and you mentioned that there's a tool or something called prettier that keeps the code neatly in order, am I right?
I'm new to coding so apologies I still don't know how to handle and utilize the IDE. 😅

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