Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I'm so sorry :( #262

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

k1ll3rwh4le15
Copy link

No description provided.

return `${dinosaurs[i].name} (${dinosaurs[i].pronunciation})\n${dinosaurs[i].info} It lived in the ${dinosaurs[i].period} period, over ${dinosaurs[i].mya[dinosaurs[i].mya.length-1]} million years ago.`
}
}return `A dinosaur with an ID of '${id}' cannot be found.`
}

Choose a reason for hiding this comment

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

You have to put together a lot of information in a very specific way to get that string right. Well done!

}
} dino[name] = length * 3.281
}return dino
}

Choose a reason for hiding this comment

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

Well done!

I would highly recommend watching your code placement and indentation—placing code right after a bracket like that can make it very difficult ot tell what scope a line occurs in.

function getDinosaursAliveMya(dinosaurs, mya, key) {
let dinosaursArray = []
for (let i = 0; i < dinosaurs.length; i++) {
if ((mya <= dinosaurs[i].mya[0] && mya >= dinosaurs[i].mya[1]) || ((mya === dinosaurs[i].mya[0])) || (dinosaurs[i].mya - mya === 1)) {

Choose a reason for hiding this comment

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

Tough logic to handle. Well done! You clearly have thought a lot about how to combine conditions.

Same note as above regarding brackets. Especially with a nested if, like with this one!

}else {
return `Dinosaur with name '${dinosaurName}' cannot be found in any rooms.`
}
}

Choose a reason for hiding this comment

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

Creative approach! Pushing them into a new array is a creative approach—it shows deep thought and experimentation. Keep it up!

Another readability principle to be conscious of: an if on the same line as the previous if's closing bracket is very easily misinterpreted as being logically connected, as with else if chains. Try to make your if placement reflect its job—form follows function.'

Copy link

@abbreviatedman abbreviatedman left a comment

Choose a reason for hiding this comment

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

Well done!

@abbreviatedman
Copy link

abbreviatedman commented Nov 4, 2022

Aside from some readability tweaks to make, and those will come with practice.

Copy link

@abbreviatedman abbreviatedman left a comment

Choose a reason for hiding this comment

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

Keep it up!

Copy link

@abbreviatedman abbreviatedman left a comment

Choose a reason for hiding this comment

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

And keep practicing.

Copy link

@abbreviatedman abbreviatedman left a comment

Choose a reason for hiding this comment

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

Looking forward to seeing you in Module 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants