Letter Changes coderbyte

Question

I've solved the problem below just would like some feedback?

The function LetterChanges(str) takes a str parameter

Replace every letter in the string with the letter following it in the alphabet (ie. 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.

function LetterChanges(str) { 
  var newString = '';
  var newCode = 0;
  var len = str.length;
  var i = 0;

  for (; i < len; i++){
  /*Using ASCII code: this if statement return true if character at
  str[i] is a - y*/
  if (str.charCodeAt(i) >= 97 && str.charCodeAt(i) <= 121) {
      //Assign ASCII code plus 1 to a variable newCode
      newCode = str.charCodeAt(i) + 1;
      /*Check to see newCode variable is a lower case vowel from ASCII 
      and if so, add capital of that vowel to newString variable, else
      use newCode variable to add character to newString*/
      if (newCode == 101) {
          newString += String.fromCharCode(69);
      } else if (newCode == 105) {
          newString += String.fromCharCode(73);
      } else if (newCode == 111) {
          newString += String.fromCharCode(79);
      } else if (newCode == 117) {
          newString += String.fromCharCode(85);
      } else {
          newString += String.fromCharCode(newCode);
      }

  /*Using ASCII code: this if statement return true if character at
  str[i] is z, in that case adding capital A*/
  } else if (str.charCodeAt(i) == 122){
      newString += String.fromCharCode(65);

  /*else just add to newString i.e uppercase(question didn't specify), 
  symbols, number...*/        
  } else {
      newString += str[i];
  }
 }
return newString; 

}

Answer 1

DRY (Don't Repeat Yourself)

You have a list of else if statements that all do the same thing. JavaScript has the .toUpperCase method so hardcoding character codes to perform capitalization is unnecessary. You can do newString += str[i].toUpperCase().

Use variables for constant values

Instead of constant values directly in your code, use a variable to store them. You know you want to capitalize vowels, so you can create an array var vowels = ['a', 'e', 'i', 'o', 'u'] to store your constants. This makes it easier to modify in the future - like if the requirements changed and now y is a vowel.

Putting these two together, your capitalization code can be pretty concise:

// vowels.indexOf(str[i]) !== -1 would work, too
if(vowels.includes(str[i]){ 
  newString += str[i].toUpperCase();
}

Answer 2

@Zanchi, Thanks for the response, I'm new to coding so really do appreciate the feedback, code looks much more concise!!

New code

function LetterChanges(str) { 
var newString = '';
var vowels = ['a','e','i','o','u']
var newCode = 0;
var len = str.length;
var i = 0;
var z = 'z';
var a = 'A';

for (; i < len; i++){
  /*Using ASCII code: this if statement return true if character at
  str[i] is a - y*/
    if (str.charCodeAt(i) >= 97 && str.charCodeAt(i) <= 121) {
      //Assign ASCII code plus 1 to a variable newCode
      //return newLetter
        newCode = str.charCodeAt(i) + 1;
        newLetter = String.fromCharCode(newCode);
      /*Loop newLetter through vowel list, add letter to newString if 
      found, else add upperCase newLetter*/
        if (vowels.includes(newLetter)) {
            newString += newLetter.toUpperCase();
        } else {
            newString += newLetter;
        }
  /*Use variables: Check to see if letter is z in which case add 'A' */
    } else if (str[i] === z){
        newString += a;

  /*else just add to newString i.e uppercase(question didn't specify), 
  symbols, number...*/        
    } else {
        newString += str[i];
    }
}
return newString; 

} 

Answer 3

I would use a hash table to keep things simpler

const LetterChanges = (str) => {
  const letters = {
    a: 'b', b: 'c', c: 'd', d: 'E', e: 'f', f: 'g', g: 'h', h: 'I', i: 'j', j: 'k', k: 'l', l: 'm', m: 'n', n: 'O', o: 'p', p: 'q', q: 'r', r: 's', s: 't', t: 'U', u: 'v', v: 'w', w: 'x', x: 'y', y: 'z', z: 'A',
  };
  return str.split('').reduce((acc, curr) => {
    return acc + letters[curr];
  }, '');
};