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

Cifrado Cesar - Andrea Chumioque #68

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

Conversation

AndreaChumioque
Copy link

@AndreaChumioque AndreaChumioque commented Nov 7, 2017

Hola, soy Andrea, espero sus comentarios sobre mi código.

Gracias.

@Gabx04
Copy link

Gabx04 commented Nov 14, 2017

@developerVilchez eslint OK.

js/app.js Outdated
// Encontramos el código de cada caracter con el método charCodeAt
origCode = msg.charCodeAt(i);
// Si el caracter es mayuscula se encuentra entre los codigos 65 y 90
if (origCode >= 65 && origCode <= 90) {
Copy link

Choose a reason for hiding this comment

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

Favor de actualizar todos los 65 a 'A'.charCodeAt(0) al igual que el 90, 97, 122 a sus respectivos valores.

js/app.js Outdated
} else if (origCode >= 97 && origCode <= 122) {
newCode = (origCode - 97 + x) % 26 + 97;
newLetter = String.fromCharCode(newCode);
msg = msg.slice(0, i) + newLetter + msg.slice(i + 1, msg.length);
Copy link

Choose a reason for hiding this comment

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

En vez de actualizar el msg una y otra vez, por que no usas una variable nueva para el mensaje nuevo y puedes usar += para aniadir chars uno por uno.

js/app.js Outdated
newCode = msg.charCodeAt(i);
// Si el caracter es mayuscula se encuentra entre los codigos 65 y 90
if (newCode >= 65 && newCode <= 90) {
if ((newCode - x % 26) < 65)
Copy link

Choose a reason for hiding this comment

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

agregar curly brackets despues del if condition.

js/app.js Outdated
if (newCode >= 65 && newCode <= 90) {
if ((newCode - x % 26) < 65)
origCode = 90 - (x % 26) + (newCode - 65) + 1;
else
Copy link

Choose a reason for hiding this comment

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

Agregar curly braces despues del else.

js/app.js Outdated
origLetter = String.fromCharCode(origCode);
msg = msg.slice(0, i) + origLetter + msg.slice(i + 1, msg.length);
} else if (newCode >= 97 && newCode <= 122) { // Si el caracter es minuscula se encuentra entre los codigos 97 y 122
if ((newCode - x % 26) < 97)
Copy link

Choose a reason for hiding this comment

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

agregar curly braces

js/app.js Outdated
if ((newCode - x % 26) < 97)
origCode = 122 - (x % 26) + (newCode - 97) + 1;
else
origCode = (newCode - x % 26);
Copy link

Choose a reason for hiding this comment

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

agregar curly braces

js/app.js Outdated
// Llamar función cipher
if (msg !== '') {
cipher(msg, 33);
break; // break para salir del bucle
Copy link

Choose a reason for hiding this comment

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

break no es necesario aqui. Generalmente se usa en switch statements o en loops. En realidad no esta haciendo nada aqui.

js/app.js Outdated
}

// Creando función para descrifrar
function decipher(msg, x) {
Copy link

Choose a reason for hiding this comment

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

Creo que no es necesario pasar el variable x.
Solo llamas esta function una vez y le das un valor de 33.

@@ -0,0 +1,90 @@
// Creando funcion Cifrado Cesar
function cipher(msg, x) {
Copy link

Choose a reason for hiding this comment

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

Segun las especificaciones, el usuario no debe poder ingresar numeros. Trate de poner numeros en mi mensaje y si pude. Favor de actualizar el code para que no pueda ingresar numeros en el mensaje

Creo que no es necesario pasar el variable x.
Solo llamas esta function una vez y le das un valor de 33.

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