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

Tiny-world #194

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Tiny-world #194

merged 2 commits into from
Aug 17, 2022

Conversation

MarharytaBoichenko
Copy link
Contributor

A Tiny JS World

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

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

@MarharytaBoichenko well done!
A couple of improvements and we are done.

];

inhabitants.forEach((item) => {
console.log(item);
Copy link
Member

Choose a reason for hiding this comment

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

Please remove debugging statements from code here and before submitting other tasks.

Comment on lines 10 to 19
species: "man",
gender: "male",
hands: 2,
legs: 2,
saying: "hello",
friends: ["Bob", "Ann", "Nick"],
};
const woman = {
name: "Ann",
species: "woman",
Copy link
Member

Choose a reason for hiding this comment

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

Men and women belong to the same species.

Comment on lines 64 to 66
const allInfoInItem = allProperties.map((prop) =>
item[prop] ? item[prop] : `no ${prop}`
);
Copy link
Member

Choose a reason for hiding this comment

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

You don't really want to spend resource on creation of locally scoped method at every loop iteration. Think of 100k iterations.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

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

@MarharytaBoichenko awesome job!

inhabitants.forEach((item) => {
print(
allProperties
.map((prop) => (item[prop] ? item[prop] : `no ${prop}`))
Copy link
Member

Choose a reason for hiding this comment

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

You don't really need so many parentheses in arrow functions, they do not really help to identify boundaries of code components, especially in one liners.

.map((prop) => (item[prop] ? item[prop] : `no ${prop}`))
.map(prop => item[prop] ? item[prop] : `no ${prop}`)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly) thanks

@OleksiyRudenko OleksiyRudenko merged commit 4884c16 into kottans:main Aug 17, 2022
@MarharytaBoichenko MarharytaBoichenko deleted the js-tiny-world branch August 17, 2022 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants