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

Conversation

@LauraJimenezH
Copy link

Hola,
Gracias por revisar mi código y darme feedback, disculpen si tengo muchos errores.

@Gabx04
Copy link

Gabx04 commented Nov 10, 2017

@developerVilchez eslint OK

Copy link
Collaborator

@nicolethenerd nicolethenerd left a comment

Choose a reason for hiding this comment

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

¡Gran trabajo! Solo tengo algunos pequeños comentarios.

js/app.js Outdated
}

// Por medio de 'prompt' le pedimos al usuario que ingrese un frase:
var order = (prompt('INGRESE UNA FRASE: '));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Las () no son necesarios, puedes escribir:
var order = prompt('INGRESE UNA FRASE: ');

js/app.js Outdated
}

// Por medio de 'prompt' le pedimos al usuario que ingrese un frase:
var order = (prompt('INGRESE UNA FRASE: '));
Copy link
Collaborator

Choose a reason for hiding this comment

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

¿Por qué esta variable se llama order? Un nombre como phrase me parece mejor.

Copy link
Author

Choose a reason for hiding this comment

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

Porque ya había utilizado ese nombre mas arriba y pensé que ya no podía utilizarlo, mejorare el nombre de mis variables gracias. :)

}

// Mensaje si ha ingresado espacios:
if (spaces === true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

=== true no es necesario. Puedes escribir solo:

if (spaces)

js/app.js Outdated
for (var re = 0; re < order.length; re++) {
// Condicional para que no se pueda ingresar numeros:
if (order.charCodeAt(re) >= 65 && order.charCodeAt(re) <= 90 || order.charCodeAt(re) > 96 && order.charCodeAt(re) < 123) {
var value = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Esta variable puede tener un nombre más descriptiva.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Por ejemplo, algo como stringContainsOnlyLetters.

js/app.js Outdated
}

// Condicional si ha ingresado solo letras sin numeros:
if (value === true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if (value)

js/app.js Outdated
}

// Mensaje si ha ingresado numeros:
if (value === false) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if (!value)

@LauraJimenezH
Copy link
Author

Gracias por el feedback, lo tomare muy en cuenta para proximos proyectos.
Muchas Gracias :)

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