-
Notifications
You must be signed in to change notification settings - Fork 10
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
NASA rover challenge #50
base: master
Are you sure you want to change the base?
Conversation
Update dev_maribel with master
} | ||
} | ||
|
||
function setup(squareSize, initialPosition, instructions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Muy buen enfoque!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aunque igual sería interesante hacer una validación de los parámetros.. para evitar sorpresas al ser tan extraña la forma de pasarnos los argumentos
machine.instructions = instructions; | ||
} | ||
|
||
function start() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bravo! Muy bien conectado y coherente con respecto a machine
👌
initialPosition = initialPosition.split(" "); | ||
machine.position.x = parseInt(initialPosition[0]); | ||
machine.position.y = parseInt(initialPosition[1]); | ||
machine.state = initialPosition[2]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Igual heading
sería más correcto que state
como nombre 👍
|
||
```javascript | ||
var machine = { | ||
squareSize: 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No pondría valores por defecto...
} | ||
|
||
function start() { | ||
machine.executeInstructions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sería interesante validar el estado d ela maquina antes de inicializar la marcha...
}, | ||
|
||
executeInstructions() { | ||
this.instructions.forEach(instr => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No se gestionan los posibles errores en la secuencia. P.j.. que se salga del mapa o similares
changePositionTo(newX, newY) { | ||
var newXIsValid = (newX >= 0 && newX < this.squareSize); | ||
var newYIsValid = (newY >= 0 && newY < this.squareSize); | ||
if (newXIsValid && newYIsValid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Muy bien la gestión de errores! 🌟
Merge master into dev_maribel
No description provided.