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

London 9 - Lovelace - Howard Sun - JavaScript-Core1 - Coursework- Week3 - #204

Open
howard-ss wants to merge 5 commits into
CodeYourFuture:mainfrom
howard-ss:main
Open

London 9 - Lovelace - Howard Sun - JavaScript-Core1 - Coursework- Week3#204
howard-ss wants to merge 5 commits into
CodeYourFuture:mainfrom
howard-ss:main

Conversation

@howard-ss

Copy link
Copy Markdown

No description provided.

update all exercises and 2 mandatories
update mandatory exercises
update 3-extra
update with review
// TODO
let i=0;
while(i<birthdays.length){
// //using includes()methods

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please remove the dead code(code is not being used). You are not applying code beatification(prettier or liniting), clean code improves the readability

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How would you fix the problem next time?
For now please refer following guide regarding linting,

https://syllabus.codeyourfuture.io/guides/code-style-guide

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.

Please remove the dead code(code is not being used). You are not applying code beatification(prettier or liniting), clean code improves the readability

Thanks for your good comments and I'll follow your comments in the future.
Because it's an exercise, I tried different solutions and commented optional solutions in the codes.

Comment thread 1-exercises/D-do-while/exercise.js Outdated
currEvenNumber += 2;
}
// required sum
return sum;

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 code is correct. Good job:)

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.

Thanks!

// i++;
//}

for (let i=0; i< 26; 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.

Keep up the good work

@rami-data-infra rami-data-infra added the reviewed A mentor has reviewed this code label Dec 14, 2022
// TODO

//1.filter()method
return allArticleTitles.filter(letters => letters.length<=65);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That's advanced, good stuff

wordNums.push(allArticleTitles[i].split(' ').length)
}
//return the array with lowest number using indexOf()method:
return allArticleTitles[wordNums.indexOf(Math.min(...wordNums))]

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 stuff

// TODO
//1. using 'regex':
let NumberArray = [];
const regex = /[0-9]/g;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a good implementation. Think if you could do this simpler way just using for loop and if else.

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.

Yes, I had optional solution with for of loop + if condition syntax btw line 91 and 110 & commented it

@rami-data-infra rami-data-infra 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.

It is very good work. I have added a few observations; please keep those in my mind for the upcoming exercises. You have understood problems thoroughly

Comment thread 2-mandatory/4-stocks.js Outdated
for (let i=0; i< closingPricesForAllStocks.length; i++){

let gap5and1=(closingPricesForAllStocks[i][4]-closingPricesForAllStocks[i][0])
priceChangeArr.push(Number(gap5and1.toFixed(2)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please think if you would really need to use Number method here

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.

Yes, it's unnecessary .
I had deleted it .

updated after code review by ramikowaluru
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