Replacing every letter in a string with the letter following it in the alphabet

Question

I am working through some of the exercises at Coberbyte. The instructions for this exercise are to replace every letter in a string with the letter following it in the alphabet (i.e., c becomes d, z becomes a), then capitalize every vowel in this new string (a, e, i, o, u) and finally return this modified string.

1. Is there a better way to advance characters alphabetically than converting to ASCII, adding one, and converting back?

2. Is there a way to do this with one loop?

3. In general, are there ways to shorten/improve the code?

var txt = prompt("Please enter some text: ");
var newTxt = "";
var newTxt2= "";

for (var i = 0; i<txt.length; i++){
  if (txt.charAt(i).contains(" ")){
    newTxt = newTxt.concat(" ");//checks for spaces
  }else if (txt.charAt(i) === "z"){
    newTxt = newTxt.concat("a");//checks for "z" and sets to "a"
  }else{
    newTxt = newTxt.concat(String.fromCharCode(txt.charAt(i).charCodeAt(0)+1));
  }//converts letter to the next letter in the alphabet
}

// checks for vowels and sets to upper case
for (var i=0; i < newTxt.length; i++){ 
  switch(newTxt.charAt(i)){
    case "a":
    case "e":
    case "i":
    case "o":
    case "u":
    newTxt2 = newTxt2 + (newTxt.charAt(i).toUpperCase());    break;
    default:
    newTxt2 = newTxt2 + newTxt.charAt(i);
    break; 
 }
}

console.log(newTxt2);

Answer 1

First, I'll note that str.charAt(n).charCodeAt(0) is better written as str.charCodeAt(n).

Strings are immutable, so it's better to work on array elements than to build them by character-by-character concatenation.

function transform(text) {
    var s = text.split('');
    for (var i = 0; i < s.length; i++) {
        // Caesar cipher
        switch(s[i]) {
          case ' ':
            break;
          case 'z':
            s[i] = 'a';
            break;
          case 'Z':     // One case you forgot to handle
            s[i] = 'A';
            break;
          default:
            s[i] = String.fromCharCode(1 + s[i].charCodeAt(0));
        }

        // Upper-case vowels
        switch(s[i]) {
          case 'a': case 'e': case 'i': case 'o': case 'u':
            s[i] = s[i].toUpperCase();
        }
    }
    return s.join('');
}

Edit note: My original answer had the for-loop written as for (var i in s) { ... }. As pointed out by @EliasVanOotegem, that is more suited for iterating over object properties than array elements. As suggested by @plalx and confirmed with jsperf, changing it to for (var i = 0; i < s.length; i++) { ... } makes it much faster. I have therefore amended my response, changing for (var i in s) to for (var i = 0; i < s.length; i++).

---

Note that your solution and the solution above blindly increments every character except spaces. If you want to increment just letters, then a replace operation would be more appropriate:

function transform(text) {
    var caesar = text.replace(/[a-z]/gi, function(c) {
        switch (c) {
          case 'z': return 'a';
          case 'Z': return 'A';
          default:  return String.fromCharCode(1 + c.charCodeAt(0));
        }
    });
    return caesar.replace(/[aeiou]+/g, function(vowel) {
        return vowel.toUpperCase();
    });
}

You could cheat on the Caesar cipher, mapping both z and Z to an uppercase A, since you know that vowels will need to be uppercased.

---

Addendum: Follow-Up Q&A

@Nathan asks in the comments…

1) I keep reading that strings are immutable and I guess this means that in my code, I'm not really concatenating, I'm replacing. I take it this is bad but I'm not sure why. I understand how your array works, just not why it is better.

If a and b are strings, when you do var c = a + b, then it creates a new string for the result (allocates memory for the length of a and b, then copies the contents of a, followed by the contents of b). If you iteratively do a concatenation for each character in a string, that works out to be O(n^2), where n is the length of the input.

In contrast, operating on an array of characters lets you modify the elements in place.

2) Both your solutions were functions. Is this so that they can be reused? The code is more self-contained and complete?

Even if you call the code just once, putting it in a function is beneficial, because it… - lets you give your code a name. Naming is important because it describes the purpose of the code. Good naming is a form of self-documentation. Awkward naming is a sign of muddled thinking. - compartmentalizes the code into small, understandable chunks. In JavaScript, variables can be scoped by function; without functions, all variables would be in the global scope, and assigning to a global variable can have side effects anywhere in your code.

Admittedly, transform(text) is not a very descriptive name, the way caesarCipher(text) is. However, the problem was a bit weird and artificial to begin with. Anyway, it's worthwhile to make it a function, to separate it from the prompting and outputting code. (Someday you might choose to use the same text transformation, but taking the text from an HTML <input> field and outputting the result to a <span>.

3) How does the function get it's argument (c)?

That's how the String.replace() function behaves. If you pass a function as the second argument to replace(), then for every match, that function will be called with the matched substring as its first parameter.

Answer 2

Something like this perhaps? I've used an object as a map for quick vowels lookups and took advantage of the fact that the replace function takes a replacement function. The whole transform is done in a single replace operation.

Note: I assume that we were only replacing lowercase letters, but it would be easy to handle all cases.

var vowels = ['a', 'e', 'i', 'o', 'u'].reduce(function (res, l) {
    res[l] = true;
    return res;
}, {});

'abcdefghijklmnopqrstuvwxyz'.replace(/[a-z]/g, function (l) {
    if (l === 'z') return 'A';
    l = String.fromCharCode(l.charCodeAt(0) + 1);
    return vowels[l]? l.toUpperCase() : l;
}); //bcdEfghIjklmnOpqrstUvwxyzA

EDIT:

Here's another way to create the map without using reduce. Like pointe out by @JanDvorak, the reduce construct might affect readability, however I like it since it allows you to define the map in a single statement, which is quite useful when you advocate using a single var statement.

var vowels = {};

['a', 'e', 'i', 'o', 'u'].forEach(function (l) {
    vowels[l] = true;
});

EDIT2:

Here's my implementation packaged within a reusable function. I know this is an exercise and that it's probably not meant to be reused, however it's worth knowing how to do it ;)

var transformLetters = (function () {
    var vowels = ['a', 'e', 'i', 'o', 'u'].reduce(function (res, l) {
            res[l] = true;
            return res;
        }, {}),
        rx = /[a-z]/g,
        replaceFn = function (l) {
            if (l === 'z') return 'A';
            l = String.fromCharCode(l.charCodeAt(0) + 1);
            return vowels[l]? l.toUpperCase() : l;
        };

    return function (s) {
        return s.replace(rx, replaceFn);
    };
})();

EDIT 3: Actually I've created a performance test that compares different solutions and it seems that using a map isin't the fastest way and it's actually slower than performing a double replace. I must say I am quite surprised. Anyway, here's the fastest implementation I could write:

PERFORMANCE TESTS (includes @200_success solutions)

var transform_switch = (function() {
  var rx = /[a-z]/g,
    replaceFn = function(l) {
      if (l === 'z') return 'A';
      l = String.fromCharCode(l.charCodeAt(0) + 1);
      switch (l) {
        case 'a':
        case 'e':
        case 'i':
        case 'o':
        case 'u':
          return l.toUpperCase();
        default:
          return l;
      }
    };

  return function(s) {
    return s.replace(rx, replaceFn);
  };
})();

Answer 3

1. No, not really. There are other ways, but nothing that is simpler or faster.

2. Yes, you can handle the z, the space and the vowels as one case, and the other characters by adding the character code:

   for (var i = 0; i<txt.length; i++){
       var c = txt.charAt(i);
       var index = " zdhnt".indexOf(c);
       if (index != -1) {
           newTxt += " AEIOU".charAt(index);
       } else {
           newTxt += String.fromCharCode(c.charCodeAt(0) + 1);
       }
   }
   

3. See 2. Alternatively, you could use a regular expression to make the vowels uppercase:

   newTxt2 = newTxt.replace(/[aeiou]+/g, function(m){ return m.toUpperCase(); });
   

(Note that the code in 2 and 3 are exclusive, if you use the code in 2, the code in 3 is not needed.)