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

WM4 - Kerim Zamir - JS Core 1 - Week 3 - #78

Open
Arbeits-Sachen wants to merge 13 commits into
CodeYourFuture:mainfrom
Arbeits-Sachen:main
Open

WM4 - Kerim Zamir - JS Core 1 - Week 3 #78
Arbeits-Sachen wants to merge 13 commits into
CodeYourFuture:mainfrom
Arbeits-Sachen:main

Conversation

@Arbeits-Sachen

Copy link
Copy Markdown

No description provided.

*note* for doesnt work
*note* const is not needed before i in for of loop
*note* const average = (closingPricesForAllStocks.map((array) => array.reduce((r, current) => r + current / array.length, 0))).map((element) => parseFloat(element.toFixed(2)));
Should be used more often

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

Good work 👏
I have just remarked that you are using a capital first letter in variables name, I'm not sure if it's the right naming convention for JS. And also variable name should be meaningful

Comment on lines +8 to +25
function evenNumbers(n)
{
let total = "0";
let calc = 0;

if (n === 0)
{
return console.log("");
}

for(let i = 1; i < n; i++)
{
calc = calc + 2;

total = total + ", " + calc;
}

return console.log(total);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The instructions said that you should use a while loop.
Consider using an array to store the values and then convert it to string using 'toString' method


for (let i = 0; i < WRITERS.length; i++)
{
console.log(WRITERS[i] + " is " + AGES[i] + " old")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
console.log(WRITERS[i] + " is " + AGES[i] + " old")
console.log(WRITERS[i] + " is " + AGES[i] + "years old")

"Tottenham Court Road"
];

for (i of tubeStations)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
for (i of tubeStations)
for (const station of tubeStations)

I think it will be better to use a meaningful variables names, it's easy to read and understand ("i" generally refers to index which is not the case here)


function getTemperatureReport(cities) {
// TODO
function getTemperatureReport(cities)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good work!

Comment on lines +12 to +21
function getRandomNumberGreaterThan50()
{
let total;
do
{
(total = generateRandomNumber());
}
while (total <= 50);

return total;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the use of 'total' as the name of the variable creates a little confusion as we don't really calculate any total in this function

Comment on lines +27 to 33
function titleWithFewestWords(allArticleTitles)
{
const shorter = (left, right) => left.length <= right.length ? left : right;

return allArticleTitles.reduce(shorter);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you please add some comments to this function, I didn't really understand it?

Comment thread 2-mandatory/4-stocks.js
Comment on lines +36 to +43
//https://jrsinclair.com/articles/2019/five-ways-to-average-with-js-reduce/
function getAveragePrices(closingPricesForAllStocks)
{
//const reduce = r => i => a => a.reduce(r, i);
//conatiner = getting items => got items.reduce => add / length .map previous => round to 2 dec
const average = (closingPricesForAllStocks.map((array) => array.reduce((r, current) => r + current / array.length, 0))).map((element) => parseFloat(element.toFixed(2)));

return average;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you please delete unnecessary comments to make the code cleaner :D

Comment thread 2-mandatory/4-stocks.js
Comment on lines +57 to +61
function getPriceChanges(closingPricesForAllStocks)
{
const PriceChange = (closingPricesForAllStocks.map((array) => array[array.length - 1] - array[0])).map((element) => parseFloat(element.toFixed(2)));

return PriceChange;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
function getPriceChanges(closingPricesForAllStocks)
{
const PriceChange = (closingPricesForAllStocks.map((array) => array[array.length - 1] - array[0])).map((element) => parseFloat(element.toFixed(2)));
return PriceChange;
function getPriceChanges(closingPricesForAllStocks)
{
const priceChange = closingPricesForAllStocks.map((array) => array[array.length - 1] - array[0])
const result = priceChange.map((element) => parseFloat(element.toFixed(2));
return result;

It's better to use to variables to make code more readable

return console.log("");
}

for(let i = 1; i < n; i++)

@Alex-Phillip Alex-Phillip Aug 19, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi Kerim, I've looked through your code and it's great! One suggestion I have here however is using a while loop instead of a for loop.
Great work!

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.

3 participants