Where is my implementation of rot13 in JavaScript going wrong?

Question

Code in question with syntax highlighting here: via Friendpaste

rot13.js:

ERRONEOUS

<script>
String.prototype.rot13 = rot13 = function(s)
 {
    return (s = (s) ? s : this).split('').map(function(_)
     {
        if (!_.match(/[A-Za-z]/)) return _;
        c = Math.floor(_.charCodeAt(0) / 97);
        k = (_.toLowerCase().charCodeAt(0) - 96) % 26 + 13;
        return String.fromCharCode(k + ((c == 0) ? 64 : 96));
     }).join('');
 };
</script>

As you can see, using quite literally a single line to attach a method to the String object a la prototyping, I'm having a map() method that I previously set up (I know for sure that that code works perfectly; it's simply iterating over each element in the array and applying the function specified in the parameter) go over each character in a string and do what I thought were the proper calculations to transform the string into it's rot13'd counterpart. I was sadly mistaken. Can anybody spot where I went wrong?

Answer 1

You could use the super-short:

s.replace(/[a-zA-Z]/g,function(c){return String.fromCharCode((c<="Z"?90:122)>=(c=c.charCodeAt(0)+13)?c:c-26);});

Answer 2

Here's a solution using replace and indexOf functions:

function rot13(s) {
  return s.replace(/[A-Z]/gi, c =>
    "NOPQRSTUVWXYZABCDEFGHIJKLMnopqrstuvwxyzabcdefghijklm"[
    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".indexOf(c) ] )
}

This is made up of:

• /[A-Z]/gi regular expression for matching only characters
• replace is used to substitute those characters
• an a replacer function written as an arrow function
• indexOf is to convert the character into a numeric lookup index
• we lookup the index in the substitution array and we're done

Answer 3

This gives correct results.

function rot13(s)
 {
    return (s ? s : this).split('').map(function(_)
     {
        if (!_.match(/[A-Za-z]/)) return _;
        c = Math.floor(_.charCodeAt(0) / 97);
        k = (_.toLowerCase().charCodeAt(0) - 83) % 26 || 26;
        return String.fromCharCode(k + ((c == 0) ? 64 : 96));
     }).join('');
 }
 
 alert(rot13(rot13("Mark this as accepted answer :)")));

Answer 4

Just because it's even shorter and also more understandable/logical:

function rot13(s) {
  return s.replace( /[A-Za-z]/g , function(c) {
    return String.fromCharCode( c.charCodeAt(0) + ( c.toUpperCase() <= "M" ? 13 : -13 ) );
  } );
}

Answer 5

Kevin M's solution is compact and elegant. It's got one tiny error, though: the regular expression used with the replace function doesn't limit substitution to alphabetic characters. The [A-z] character range includes punctuation characters ([\] ^ _ `), which will be swapped for letters when they should be left alone.

The fixed version looks like this:

function r(a,b){return++b?String.fromCharCode((a<"["?91:123)>(a=a.charCodeAt()+13)?a:a-26):a.replace(/[a-zA-Z]/g,r)}

It's still only 116 bytes. Remarkably small and quite clever.

(Sorry for the full answer posting; I'm still lacking the 50 rep required to post this as a comment to Kevin's excellent answer.)

Answer 6

Here's a version that's 80-columns, doesn't update string.prototype, well indented and reasonably short.

function rot13(str) {
  return str.replace(/[a-zA-Z]/g, function(chr) {
    var start = chr <= 'Z' ? 65 : 97;
    return String.fromCharCode(start + (chr.charCodeAt(0) - start + 13) % 26);
  });
}

And an example showing it is working:

rot13('[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ]')
"[nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM]"
rot13(rot13('[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ]'))
"[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ]"

Answer 7

var rot13 = String.prototype.rot13 = function(s)
{
  return (s = (s) ? s : this).split('').map(function(_)
  {
    if (!_.match(/[A-Za-z]/)) return _;
    c = _.charCodeAt(0)>=96;
    k = (_.toLowerCase().charCodeAt(0) - 96 + 12) % 26 + 1;
    return String.fromCharCode(k + (c ? 96 : 64));
  }
  ).join('');
};

alert('abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'.rot13());
yields nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM

Mixing zero-based and one-based indices for the lose. I blame Netscape.

Answer 8

One-liner that weighs in at 116 bytes:

function r(a,b){return++b?String.fromCharCode((a<"["?91:123)>(a=a.charCodeAt()+13)?a:a-26):a.replace(/[a-zA-Z]/g,r)}

Usage:

r('The Quick Brown Fox Jumps Over The Lazy Dog.');

Answer 9

There is still room for enhancement, checking (c<="Z") is actually a check against the codepoint (that we need later on), following that idea gives us a win !

//versus Kevin M's style : 115 chars (vs 116)
//102 chars with nodejs Buffer (see below)

function r(a,b){return++b?String.fromCharCode(((a=a.charCodeAt())<91?78:110)>a?a+13:a-13):a.replace(/[a-zA-Z]/g,r)}
//nodejs style
function r(a,b){return++b?Buffer([((a=Buffer(a)[0])<91?78:110)>a?a+13:a-13]):a.replace(/[a-zA-Z]/g,r)}

//versus Ben Alpert style : 107 chars (vs 112)
//93 chars with nodejs Buffer (see below)

s.replace(/[a-zA-Z]/g,function(a){return String.fromCharCode(((a=a.charCodeAt())<91?78:110)>a?a+13:a-13)});
//nodejs style
s.replace(/[a-zA-Z]/g,function(a){return Buffer([((a=Buffer(a)[0])<91?78:110)>a?a+13:a-13])})

// Same code, formated for production

String.prototype.rot13 = function() {
  return this.replace(/[a-zA-Z]/g, function(a){
    return String.fromCharCode(((a=a.charCodeAt())<91?78:110)>a?a+13:a-13);
  });
}

In nodejs, you can use Buffer to cast/serialize codepoints, e.g. :

var a=65;
""+Buffer([a])           == "A" // note that the cast is done automatically if needed
String.fromCharCode(a)   == "A"

var b="A";
Buffer(a)[0]             == 65
a.charCodeAt()           == 65

Answer 10

My golfed version is 82 bytes long (vs. Ben Albert, which is 35% heavier, but inspired mine):

S.replace(/[a-z]/gi,c=>String.fromCharCode((c=c.charCodeAt())+((c&95)>77?-13:13)))

Differences:

• case-insensitive to catch only English alphabet.
• arrow functions without return and braces.
• delete parameters from charCodeAt.
• test against code insted of string.
• doing +13-26=-13.
• test uppercased (&95) against 77 (78+13=91, overflow).

---

Extra: If you want to perform ROT5 on digits, add: .replace(/\d/gi,c=>(c>4?-5:5)+c*1)

Answer 11

Combining various techniques here, I came up with this 78 character JavaScript ES6 function, which works on Node:

rot13=s=>s.replace(/[a-z]/ig,c=>Buffer([((d=Buffer(c)[0])&95)<78?d+13:d-13]));

Answer 12

Here’s a JavaScript library that performs ROT-n letter substitution: https://github.com/mathiasbynens/rot

rot is a JavaScript library that performs rotational letter substitution. It can be used to shift any ASCII letters in the input string by a given number of positions in the alphabet. To ROT-13 the string 'abc', for example:

// ROT-13 is the default
rot('abc');
// → 'nop'

// Or, specify `13` explicitly:
rot('abc', 13);
// → 'nop'

Answer 13

This is in no way trying to compete with the excellent stuff here, as you see I can't comment but I have my own novice attempt to write this in JS and getting it to work before I read more elegant solutions here - I'm going to share it here.

I tried to write it with indexOf, a switch, by adding 13, by String.fromCharCode() and CharCodeAt(). They were getting too long - the helper function in this one is unnecessary but this was my shortest : )

function rot13(string) { 
  var result = '', 
      store,
      str = string.toLowerCase();  

  //helper function
  function strgBreak(a){
    var result = [];
    return result = a.split('');
  }

  //rot13 arrays
  var alphArr = strgBreak('abcdefghijklmnopqrstuvwxyz');
  var inverseArr = strgBreak('nopqrstuvwxyzabcdefghijklm');

 for ( var i = 0; i < str.length; i++ ) {
     if (alphArr.indexOf( str[i] ) !== -1) {
        result += inverseArr[ alphArr.indexOf( str[i] ) ];
    } else result += str[i];
  }
 return result.toUpperCase(); 
}

Answer 14

Here's a modern approach to the ROT13 substitution cipher:

const ROT13 = s =>
  s.replace(/[a-z]/gi, c =>
    String.fromCharCode(c.charCodeAt() + 13 - 26 * /[n-z]/i.test(c)));

console.log(ROT13('The quick brown fox jumps over 13 lazy dogs.'));

---

The result of the test case above is:

Gur dhvpx oebja sbk whzcf bire 13 ynml qbtf.

Answer 15

CoffeeScript version of @ben-alpert 's answer:

string.replace /[a-zA-Z]/g, (c) -> String.fromCharCode if (if c <= 'Z' then 90 else 122) >= (c = c.charCodeAt(0) + 13) then c else c - 26

Or as function:

ROT13 = (string) -> string.replace /[a-zA-Z]/g, (c) -> String.fromCharCode if (if c <= 'Z' then 90 else 122) >= (c = c.charCodeAt(0) + 13) then c else c - 26
ROT13('asd') # Returns: 'nfq'

Answer 16

My favourite and simple to understand version of decision ROT13

function rot13(message) {
  var a = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
  var b = "nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM"
  return message.replace(/[a-z]/gi, c => b[a.indexOf(c)])
}

a - classic alphabet, b - prepered dictionary with 13th replacement, return result with simple RegEx replacement

Answer 17

While I really like the RegEx solution, I primarily undertook the project to see if I could get it done. Glad to report that I finally did manage to do so:

String.prototype.rot13 = rot13 = function(s)
 {
    return (s ? s : this).split('').map(function(_)
     {
        if (!_.match(/[A-za-z]/)) return _;
        c = Math.floor(_.charCodeAt(0) / 97);
        k = (_.toLowerCase().charCodeAt(0) - 83) % 26 || 26;
        return String.fromCharCode(k + ((c == 0) ? 64 : 96));
     }).join('');
 }