A small custom encryption / decryption script

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.

This is a simple variant of the caesar-cipher, namely a vigenere-cipher 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('');

StackExchange Code Review Q#127454, answer score: 3