I've written a very simple cryptor and decryptor for numbers, based on the idea of atbash cypher. The code works fine, but it seems to me that it is really ugly. Any ideas to make it look professional? And, any ideas to improve it?

function atbash(n) {

var number= [];

atbashed = [5, 6, 7, 8, 9, 0, 1, 2, 3, 4];

len = n.toString().length;

for (i=0; i<len; i++){

    number[i] = atbashed[parseFloat(n.toString().charAt(i))];

}

return number.join('');

}

function numsum(n) {

var number = 0;

len = n.toString().length;

for (i=0; i<len; i++) {

    number += parseFloat(n.toString().charAt(i));

}


return number;

}

function crypt(n){

return n.toString().length + atbash(n) + numsum(n);

}

function decrypt(n){

len = n.toString().charAt(0);
code = n.toString().substr(1, len);
crc = n.toString().substr(parseFloat(len)+1, n.toString().length);

if (numsum(atbash(code)) == crc) {

    return atbash(code);

} else {

    return 'something is funky!';

}

}

The idea of the script is this: the first character indicates how long the crypted number is. The numbers after that crypted, is used as a CRC to check that the number hasn't changed. It simply is the sum of the uncrypted number.

  • Sounds like a question for stackoverflow.com instead - its more programming oriented. Richard Grevers over 7 years ago

1 answer

danwellman 5600
3
points

At a basic level it looks as though there are at least a couple of optimisations you can make, first of all, in each function you are declaring several variables but only the first variable is initiliased with the var keyword. This means that the first variable will be in the function's scope, while the other variables will have global scope (variables defined without the var keyword are automaically global). Instead of using:

var number= [];

atbashed = [5, 6, 7, 8, 9, 0, 1, 2, 3, 4];

len = n.toString().length;

Instead use:

var number= [],
atbashed = [5, 6, 7, 8, 9, 0, 1, 2, 3, 4],
len = n.toString().length;

With this code, a comma is used to separate the variables instead of a semi-colon so all of the variables will use the initial var keyword and all variables will be scoped locally to the function. Global vars should be avoided wherever possible in JS to minimise variable conflict when your code is used as part of a wider codebase.

Also you are making the i variable in your for loops global. While this will probably not have any conflicts, it is still best practice to use for (var i=0; i<len; i++){ instead.

In the last function you're using a standard if...else statement, which will work, but there is a better way - use the ternary statement instead of the standard if...else when there is only one action to complete within each branch of the code, so instead of using

if (numsum(atbash(code)) == crc) {

    return atbash(code);

} else {

    return 'something is funky!';

}

You could use

return (numsum(atbash(code)) == crc) ? atbash(code) : 'something is funky!';

This is much more compact but will do the job just as effectively. You only need to use the return statement once instead of in each branch. The condition to check appears first in brackets, the if...true part is defined following the ternary character ? and the else condition appears after the colon.

Other than that, your code looks quite good :D

Answered over 7 years ago by danwellman
  • Why is this being down-voted? There is absolutely *nothing* wrong with what I have suggested and it is quite a detailed answer. Considering the poster's function have only a line or two of code in them there is little that can be optimised danwellman over 7 years ago
  • I think this place has begun to suffer from all them forum-surfers who 1) ask questions before thinking, not posting any links or resources and 2) thinks the answers here are about opinition and not fact. I've been voted down simply because my answer contained a methology they don't use and therefore it's "bad". How to stop this trend I wonder... Jens Hedqvist over 7 years ago
  • I'd like to hope this was not the case for the majority of users on here. I think all we can do is support correct answers (or answers that are not clearly incorrect or unrelated)... danwellman over 7 years ago
  • Sorry guys, I've got down-vote-costs-one-point almost ready to go, this should reduce these annoying instances. David Smalley over 7 years ago
  • An excellent solution :) I assume people with no rep points will be unable to down-vote in that case danwellman over 7 years ago