HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMajor

ROT13 JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptrot13stackoverflow

Problem

I wrote a ROT13 cipher script rotating uppercase letters only. Can this function be improved?

ROT13 Wikipedia article

function rot13(str) {

var codeA = "A".charCodeAt(0);
var codeN = "N".charCodeAt(0);
var codeZ = "Z".charCodeAt(0);
var newArr = [];

for(var i =0; i=codeA && code=codeN)
            newArr.push(String.fromCharCode(code-13));
        else
            newArr.push(String.fromCharCode(code+13));
    }else{
        newArr.push(str[i]);}
    }
     return newArr.join("");
}

Solution

The mapping is so simple, I prefer to do a more literal implementation of the wiki explanation:

function rot13(str) {
  var input     = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
  var output    = 'NOPQRSTUVWXYZABCDEFGHIJKLMnopqrstuvwxyzabcdefghijklm';
  var index     = x => input.indexOf(x);
  var translate = x => index(x) > -1 ? output[index(x)] : x;
  return str.split('').map(translate).join('');
}


Here the code is expressing just two rules:

  • If it's a non-letter, don't translate it.



  • If it's a letter, translate it by mapping its physical position in the input to its physical position in the output.



This avoids charCodes, loops, and nested if statements, all of which are implementation noise and distract from the essence of the program.

While this still executes in O(n), the constant factor on n is high, because in the worst case we have to look through 52 input letters to find each character. In practice, however, this makes very little difference: https://jsperf.com/rot13comparison
Even Faster Variation

Nevertheless, it's possible to avoid the high constant factor on n if you were in a situation where squeezing out every last drop of performance mattered. And indeed, you could probably squeeze out even more by replacing the split/map with a for loop. But again, these kinds of optimizations are rarely needed, and concentrating on clean code should usually take precedence.

Here's a slight variation on the same theme, which creates a lookup dictionary between the input and output letters using an object, and then uses that object to do the translation:

function rot13Fast(str) { 
    return str.split('').map(x => rot13Fast.lookup[x] || x).join('')
  }
  rot13Fast.input  = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'.split('')
  rot13Fast.output = 'NOPQRSTUVWXYZABCDEFGHIJKLMnopqrstuvwxyzabcdefghijklm'.split('')
  rot13Fast.lookup = rot13Fast.input.reduce((m,k,i) => Object.assign(m, {[k]: rot13Fast.output[i]}), {})

Code Snippets

function rot13(str) {
  var input     = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
  var output    = 'NOPQRSTUVWXYZABCDEFGHIJKLMnopqrstuvwxyzabcdefghijklm';
  var index     = x => input.indexOf(x);
  var translate = x => index(x) > -1 ? output[index(x)] : x;
  return str.split('').map(translate).join('');
}
function rot13Fast(str) { 
    return str.split('').map(x => rot13Fast.lookup[x] || x).join('')
  }
  rot13Fast.input  = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'.split('')
  rot13Fast.output = 'NOPQRSTUVWXYZABCDEFGHIJKLMnopqrstuvwxyzabcdefghijklm'.split('')
  rot13Fast.lookup = rot13Fast.input.reduce((m,k,i) => Object.assign(m, {[k]: rot13Fast.output[i]}), {})

Context

StackExchange Code Review Q#132125, answer score: 26

Revisions (0)

No revisions yet.