I have made a small script for encrypting data. Now I'm wondering how efficient it is for normal users who aren't programmers and for people who are programmers.

I mean if you don't have this script there is no way to find out what the text is right?

function cipher(message, action) {
  var text = message;
  var encrypted = "";

  for(var i = 0; i < text.length; i++) {
    var ASCII = text[i].charCodeAt(0);
    var n = null;

    if(i % 2 == 0) {
      n = action == 'encrypt' ? ASCII + 4 : ASCII - 4;

    else if(i % 2 == 1) {
      n = action == 'encrypt' ? ASCII + 7 : ASCII - 7;

    var s = String.fromCharCode(n);

        encrypted += s;;
   return encrypted;

console.log(cipher("Hello World", 'encrypt'))

If you were to make it more secure, what would you add? I was thinking of a complete custom random mapping for each letter but retrieving it after decryption is something I can't figure out yet.

  • \$\begingroup\$ First rule of cryptography: Don't roll your own crypto. \$\endgroup\$
    – Mast
    Commented May 4, 2016 at 9:10
  • \$\begingroup\$ @Mast haha ya. Im just learning so its ok : ) \$\endgroup\$
    – Asperger
    Commented May 4, 2016 at 9:27

1 Answer 1


This is a simple variant of the , namely a with the fixed key 'EH'. Any variant of these ciphers is considered a toy-grade cipher that is trivially crackable if the attacker has gathered enough ciphertext to analyze the letter frequencies. There is no way to make this class of ciphers secure, except to allow an insanely long and random key to be used, in which case it becomes a one-time pad.

If, despite acknowledging that it cannot be made secure, you still wanted to try to make some improvements to this code as a learning exercise, you could…

  • Allow the key to be specified as a parameter rather than hard-coded as alternating between +4 and +7
  • Eliminate the text variable
  • Write var ASCII = text[i].charCodeAt(0); as var unicode = message.charCodeAt(i);
  • Avoid repeated string concatenation with encrypted += s;;. Since strings in JavaScript are immutable, you are actually creating a new string and copying the entire old string each time you want to append one character. Instead, you should use Array.join():

    var encrypted = new Array(message.length);
    for (var i = 0; i < message.length; i++) {
        encrypted[i] = …;
    return encrypted.join('');
  • \$\begingroup\$ Minor quibble: Array.join is undefined; Perhaps Array.prototype.join or Array#join is more correct. \$\endgroup\$ Commented May 4, 2016 at 11:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.