Cesar's Cypher method repeating the last letter of each word

Question

I'm making Cesar's Cypher that shifts the letters 13 places. It works, but the last letter of each word is repeated. I know there is probably a more efficient way to write this, but here is my code.

function rot13(str) {
  var letters = ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"];
  var word = "";
  var z = 30;
  for (i = 0; i < str.length; i++) {
    if (str.substring(i, i + 1) === " ") {
      word = word + str.substring(i, i + 1);
    }

    for (x = 0; x < letters.length; x++) {
      if (str.substring(i, i + 1) === letters[x]) {
        z = x; //18
      }
    }

    var n = z + 13; //31
    if (n >= 26) {
      word = word + letters[13 - (26 - z)];
    } else if (n < 26)
      word = word + letters[13 + z];
  }

  return word;
}


rot13("LBH QVQ VG!");

For example LBH QVQ VG! prints out YOU UDID DITT instead of YOU DID IT!. Also, I'm new to javascript so if any syntax looks off please correct me.

You'd probably have an easier time here using a look-up table of the form `{ a: "m", b: "n" ... }` where you can do a straight up substitution on each letter with `replace` using a function. — tadman, Aug 08 '17 at 19:33
Possible duplicate of [Where is my one-line implementation of rot13 in JavaScript going wrong?](https://stackoverflow.com/questions/617647/where-is-my-one-line-implementation-of-rot13-in-javascript-going-wrong) — Maciej Kwas, Aug 08 '17 at 19:35
If you really want to learn programming, this seems like a nice opportunity to learn how to use a debugger. — Bane Bojanić, Aug 08 '17 at 19:46

Stephan · Answer 1 · 2017-08-08T19:50:56.367

Your first problem is that you should continue; the for loop if you detect a space (otherwise your last used character gets appended a second time):

if (str.substring(i, i + 1) === " ") {
  word += " ";
  continue;
}

Your second problem is that you don't care if the current character isn't found in your letters array, in that case your code simply uses the last z.

Just as a sidenote: In your position I would take a look at JS' built-in functions, for example split, indexOf, charCodeAt, fromCharCode, which can simplify your life quite a bit.

score 0 · Answer 2 · answered Aug 08 '17 at 19:47

The problem is that when you find a space and copy it to the result, you don't skip over the rest of the code that processes the character. Since the loop that sets z never finds a match, it uses the value of z from the previous iteration of the loop, so it duplicates the last character of the previous word.

The same thing happens if there's a punctuation character. In this case, the code that tests for space doesn't copy it to the result, then you go into the loop that looks for the character in letters, it doesn't find it, so it again uses the previous value of z.

A simple solution for both is to check whether the current character is in letters, rather than checking specifically for space.

BTW, you can use str[i] or str.charAt(i) to get the current character in the string, rather than str.substring(i, i+1). And to find an element of an array, use letters.indexOf(char) instead of an explicit loop.

function rot13(str) {
  var letters = ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"];
  var word = "";
  var z = 30;
  for (i = 0; i < str.length; i++) {
    var char = str[i];
    z = letters.indexOf(char);
    if (z == -1) {
      word += char;
      continue;
    }

    var n = z + 13; //31
    if (n >= 26) {
      word = word + letters[13 - (26 - z)];
    } else if (n < 26)
      word = word + letters[13 + z];
  }

  return word;
}


console.log(rot13("LBH QVQ VG!"));

And instead of your if statement at the end, you can use the modulus operator:

word += letters[n % letters.length];

Cesar's Cypher method repeating the last letter of each word

2 Answers2