AraceliGS #5
AraceliGS #5
Conversation
nicolethenerd
left a comment
nicolethenerd
left a comment
There was a problem hiding this comment.
¡Bien hecho!
Tengo algunos comentarios de la organización de tu código. Estaba muy impresionado con su uso de la guía de estilo (https://github.com/Laboratoria/js-style-guide#agrega-espacios-a-cada-lado-de-los-operadores).
| <title>Cifrado César</title> | ||
| </head> | ||
| <body> | ||
| <script type="text/javascript" src = "js/app.js"></script> |
There was a problem hiding this comment.
borra los espacios alrededor del =
| // Creando función cipher | ||
|
|
||
| function cipher(str) { | ||
| var index1 = []; // [65, 66, 67] || [97, 98, 99] |
There was a problem hiding this comment.
No es obvio en tu código porque los números 65 y 97 son importantes. Puedes usar constantes, como
var UPPERCASE_A = 65;
var LOWERCASE_A = 97;
Y en otras partes de tu código, como la línea 12, puedes usar UPPERCASE_A en lugar de 65.
| var index1 = []; // [65, 66, 67] || [97, 98, 99] | ||
| var indexAscii = []; // [72, 73, 74] || [104, 105, 106] | ||
| var letters1 = []; // ['H', 'I', 'J'] || ['h', 'i', 'j'] | ||
| if (str === str.toUpperCase()) { |
There was a problem hiding this comment.
Tu código no funciona si str contiene una combinación de letras mayúsculas y minúsculas (por ejemplo, probar el input 'Hola')
There was a problem hiding this comment.
Es mejor poner el if dentro del for loop, y probar cada letra para ver si es mayúsculas o minúsculas - no probes el string completo
| var indexAscii = []; // [72, 73, 74] || [104, 105, 106] | ||
| var letters1 = []; // ['H', 'I', 'J'] || ['h', 'i', 'j'] | ||
| if (str === str.toUpperCase()) { | ||
| for (var i = 0; i < str.length; i++) { |
There was a problem hiding this comment.
Este loop y la variable index1 no son necesarios. Puedes usar str.charCodeAt(j) en la línea 12.
| for (var i = 0; i < str.length; i++) { | ||
| index1.push(str.charCodeAt(i)); | ||
| } | ||
| for (var j = 0; j < index1.length; j++) { |
There was a problem hiding this comment.
Puedes repetir la variable indice de un loop - puedes usar 'i' en todos tus loops, excepto cuando hay un loop dentro de otro loop.
| } | ||
| } | ||
|
|
||
| if (str === str.toLowerCase()) { |
There was a problem hiding this comment.
Las líneas 21-37 no son necesarios si mueves el if dentro del loop, como describí anteriormente.
|
|
||
| // Creando función decipher | ||
|
|
||
| function decipher(str) { |
There was a problem hiding this comment.
Tengo comentarios similares para esta función:
El if en la línea 50 y las líneas 67-82 no son necesarios - es mejor probar si las letras son mayúsculas o minúsculas dentro del loop.
| } | ||
|
|
||
| if (str === str.toLowerCase()) { | ||
| for (var num1 = 0; num1 < str.length; num1++) { |
There was a problem hiding this comment.
Esta loop no es necesario - puedes usar str.charCodeAt en el loop segundo.
| } | ||
|
|
||
|
|
||
| for (var num3 = 0; num3 < indexAscii.length; num3++) { |
There was a problem hiding this comment.
También este loop puede ser combinado con el segundo loop - no necesitas el arreglo indexAscii - puedes hacer el string nuevo directamente.
2 participants
No description provided.