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.