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

WM4 - Azin Yadegari - JavaScript -Core-1-Week3 - #102

Open
AzinYad wants to merge 5 commits into
CodeYourFuture:mainfrom
AzinYad:main
Open

WM4 - Azin Yadegari - JavaScript -Core-1-Week3#102
AzinYad wants to merge 5 commits into
CodeYourFuture:mainfrom
AzinYad:main

Conversation

@AzinYad

@AzinYad AzinYad commented Aug 19, 2022

Copy link
Copy Markdown

No description provided.

@Dawit-Dev Dawit-Dev 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 job. Your codes are very clean and simple to comprehend. maintain your good work

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

Well done Azin.

let i=0
let sum=0
do {
sum+=(i*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.

it looks simple and smart

function getTemperatureReport(cities) {
// TODO
let tempRepo=[]
for (i=0 ; i < cities.length ; 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.

I also often forget to declare variable -i in loops))

// TODO
let titleWithNum=[]
for (const title of allArticleTitles){
if (/\d/.test(title)){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

test() - interesting method, I'll keep it in mind

function getTemperatureReport(cities) {
// TODO
let tempRepo=[]
for (i=0 ; i < cities.length ; 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.

What's missing from this line of code?

// TODO
let acceptedTitles=[]
let lengthCheck=0
for ( i=0 ; i < allArticleTitles.length ; 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.

What's missing from this line of code?

let wordCountOFTitles=[]
let i=0
do {
let wordCount= allArticleTitles[i].trim().split(" ").length

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 job calling .trim() 👍 why did you do that?

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,to remove any blank space from both end , & prevent counting extra word :)

}
while(i < allArticleTitles.length)

minValue= Math.min(...wordCountOFTitles)

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
let sum=0
let average=0
let j=0
do{

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 chose a do while loop for this?

Comment thread 2-mandatory/4-stocks.js
let priceChangeArray=[]
for(i=0 ; i< closingPricesForAllStocks.length ; i++){
let priceChange=0
priceChange =parseFloat((closingPricesForAllStocks[i][(closingPricesForAllStocks[i].length -1)]-closingPricesForAllStocks[i][0]).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.

Can you think of a way to make this line of code more clear?

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

Good effort, don't forget to format your code before creating a PR

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.

4 participants