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

solution #2382

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

solution #2382

wants to merge 3 commits into from

Conversation

cardinal1991
Copy link

No description provided.

src/makeRobot.js Outdated
@@ -39,6 +39,73 @@
*/
function makeRobot(name, wheels, version) {
// write code here
const Robot = {
Copy link

Choose a reason for hiding this comment

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

capital first letter names are reserved for JS classes - please use plain object - robot

src/makeRobot.js Outdated
Comment on lines 103 to 105
name,
wheels,
version,
Copy link

Choose a reason for hiding this comment

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

properties are always first in each object, then goes functions (methods), please move those on top

src/makeRobot.js Outdated
Comment on lines 51 to 54
coords: {
x: 0,
y: 0,
},
Copy link

Choose a reason for hiding this comment

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

properties are always first in each object, then goes functions (methods), please move those on top

src/makeRobot.js Outdated

return this;
},
evacuate: function evacuate() {
Copy link

Choose a reason for hiding this comment

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

checklist point 2

src/makeRobot.js Outdated
Comment on lines 89 to 91
if (step < 0) {
this.coords.x = this.coords.x;
}
Copy link

Choose a reason for hiding this comment

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

you can omit that - it does nothing

src/makeRobot.js Outdated
},
goLeft(step = 1) {
if (step < 0) {
this.coords.x = this.coords.x;
Copy link

Choose a reason for hiding this comment

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

you can omit that - it does nothing

src/makeRobot.js Outdated
},
goBack(step = 1) {
if (step < 0) {
this.coords.y = this.coords.y;
Copy link

Choose a reason for hiding this comment

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

you can omit that - it does nothing

src/makeRobot.js Outdated
},
goForward(step = 1) {
if (step < 0) {
this.coords.y = this.coords.y;
Copy link

Choose a reason for hiding this comment

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

you can omit that - it does nothing

@cardinal1991 cardinal1991 requested a review from choeqq July 27, 2023 11:15
Copy link

@JoyCoffeeAddict JoyCoffeeAddict left a comment

Choose a reason for hiding this comment

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

You could add some spacing to improve readability, other than that great job!

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.

3 participants