Ruby function that changes each letter for a string and also capitalizes vowels?

Question

I am trying to do a challenge on Coderbyte. The question states:

Have the function LetterChanges(str) take the str parameter being passed and modify it using the following algorithm. 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.

Here is my code:

hash = {"a" => 1,"b" => 2,"c" => 3,"d" => 4,"e" => 5,"f" => 6,"g" => 7,"h" => 8,"i" => 9,"j" => 10, "k" => 11,"l" => 12,"m" => 13,"n" => 14,"o" => 15,"p" => 16,"q" => 17,"r" => 18,"s" => 19,"t" => 20,"u" => 21,"v" => 22,"w" => 23,"x" => 24,"y" => 25,"z" => 26}
def LetterChanges(str)
  chars = str.split("")
  newstr = Array.new
  i = 0
  newletter = 0
  while i <= chars.length
    if hash.has_key?(chars[i]) == true    #I think this is where the problem is
      newletter = hash[chars[i]] + 1
      newstr.push(has.key(newletter))
        if newstr[i].include?('a','e','i','o','u')
          newstr[i].upcase!
        end
    else
      newstr.push(chars[i])
    end
    i += 1
  end
  return newstr
end

It keeps saying there is an error with 'has_key?'. I also tried using '.include?' and 'chars[i] =~ [a-zA-Z]' but all return an error. I'm not sure why it isn't accepting any of these methods/regex. If you do decide to answer using regular expressions, please explain in details because they still confuse me a little.

Thanks in advance.

***EDIT: I have taken all of your advice and thought I had a working code, but apparently not. =/

I get this error: (eval):8: undefined method key' for #<Hash:0x149bf0> (NoMethodError) from (eval):4:ineach' from (eval):4:in `LetterChanges' from (eval):18

 1 def LetterChanges(str)
 2 hash = {"a" => 0,"b" => 1,"c" => 2,"d" => 3,"e" => 4,"f" => 5,"g" => 6,"h" => 7,"i" => 8,"j" => 9, "k" => 10,"l" => 11,"m" => 12,"n" => 13,"o" => 14,"p" => 15,"q" => 16,"r" => 17,"s" => 18,"t" => 19,"u" => 20,"v" => 21,"w" => 22,"x" => 23,"y" => 24,"z" => 25}
 3 newstr = Array.new
 4 newletter = 0
 5 str.each do |i|
 6   if hash.has_key?(str[i]) 
 7     newletter = hash[str[i]] + 1
 8     newletter = 0 if newletter == 25
 9     newstr.push(hash.key(newletter))
 10    newstr[i].upcase! if newstr[i] =~ /[aeiou]/
 11  else
 12     newstr.push(str[i])
 13  end
 14 end
 15 return newstr.to_s
 16end

Answer 1

Nate Beers' two-line solution above is almost perfect but doesn't seem to wrap z -> a. Unfortunately, I don't have the reputation to leave this correction as a comment.

You can solve this by modifying his code to:

def LetterChanges(str)
   str.tr!('a-z','b-za')
   str.tr!('aeiou','AEIOU')
   return str 
end

Answer 2

There is a pretty easy two line way to do that....

def LetterChanges(str)
   str.tr!('a-y','b-z')
   str.tr!('aeiou','AEIOU')
   return str 
end

Answer 3

First of all I don't know Coderbyte but it's common practice in Ruby to use snake_case for naming methods (but that's not important here)

The biggest point is this: As soon as you get a 'z' on your input string, you're gonna have a baaad time: In the Line newletter = hash[chars[i]] + 1 you correctly determine the letter's ID (hash[chars[i]], which results in 26) and add 1. However, when converting back to a letter (in the line which I assume should be like this: newstr.push(hash.key(newletter)) - you mistyped hash) you reference a value (27) which does not exist in the hash!

Next, you use include? the wrong way around. Note that include? only takes one argument. We actually have two ways to check if a char is a vovel:

letter.upcase! if %{a e i o u}.include?(letter)

(note that %w{a e i o u} constructs an array containing all the letters). Or:

letter.upcase! if letter =~ /[aeiou]/

(=~ checks if letter matches the RegEx /[aeiou]/)

This should help you get it working!

Here's a working example (but try solving it yourself first!!!) https://gist.github.com/mhutter/8678067

---

Here are some Tips:

• When using Hashes with numbers, it's good practice to make them Zero-Indexed: the lowest number (in your case 'a' => 1 should always be 0).

• The % operator returns the remainder of a division. This is especially handy when incrementing indexes of an array. array[(i+1) % array.length] will always get you a valid value.

• Your line if hash.has_key?(chars[i]) == true will work as expected, BUT Hash#has_key? already returns true or false so no need for checking for true. Simply write if hash.has_key?(chars[i]).

• Strings can be accessed like this: "Foobar"[3], so there's no need splitting them up.

BUT if you do so, you can then do this:

chars = str.split ""
chars.each do |char|
  # do something with char
end

however, if you do NOT split your string :), you can replace this:

i = 0
while i <= chars.length
  # current char is char[i]

with this:

0.upto(chars.length) do |i|
  # current char is char[i]

Constructs like this:

if something?
  do_something
end

can be shortened to this:

do_something if something?

Answer 4

I think you meant to put newstr.push(hash.key(newletter)) not newstr.push(has.key(newletter)). Also, you declare hash as a class variable and not an instance variable. Try puts hash inside the function and you'll see what I mean.

I would also suggest changing your method definition to follow a few ruby conventions. In ruby, we usually use snake case unless it's a class, i.e. def letter_changes(str).

We also don't use while if we don't have to. You can change that code to look like this

chars.each_with_index do |current_char, i|
  if hash.has_key?(current_char)
    newletter = hash[current_char] + 1
    newstr.push(hash.key(newletter))
    newstr[i].upcase! if newstr[i].include?('a','e','i','o','u')
  else
    newstr.push(current_char)
  end  
end

And we also don't usually return explicitly unless it's from within an if/else/unless/case statement.

Answer 5

I believe your problem with has_key? is because hash is not in the scope of that method. Move the hash={blah} to the line after the def, and also take snowe2010's advice on refactoring the code to make it more ruby like.

Answer 6

Is this a typo?

newstr.push(has.key(newletter)) instead of hash.key

If that's not the problem, post the error with 'has_key?'.