Learning Ruby Caesar Cipher

Question

I am trying to learn Ruby and made a simple Caesar cipher program and I was wondering if the following code follows the 'ruby way' or the way most ruby users would do it

# Implements the classes caesar cipher encryption and decryption.
class CaesarCipher
 def initialize(shift)
 @shift = shift
 end
 def encrypt(str)
 str_array = str.split("")
 str_array.map! { |char| round_over(char) }.join
 end
 def decrypt(str)
 str_array = str.split("")
 str_array.map! { |char| round_down(char) }.join
 end
 private
 def round_over(char)
 if char.ord + @shift > "z".ord
 (char.ord + @shift - 26).chr
 elsif char.ord + @shift > "Z".ord && char.ord <= "Z".ord
 (char.ord + @shift - 26).chr
 else
 (char.ord + @shift).chr
 end
 end
 def round_down(char)
 if char.ord - @shift < "a".ord && /[[:lower:]]/.match(char) # checks for lowercase
 (char.ord - @shift + 26).chr
 elsif char.ord - @shift < "A".ord
 (char.ord - @shift + 26).chr
 else
 (char.ord - @shift).chr
 end
 end
end

Answer 1

That's a pretty good start, however I noticed that your code only works on letters. For non-alphabetic characters it returns confusing results e.g. encrypting and decrypting ! gives you ;. I can think of three good ways to handle non-alphabetic characters:

• Ignore them
• Use ASCII codes to shift them as well (so Z shifted right one becomes [)
• Raise an error

Any of those approaches will ensure that decrypting and encrypting are inverses (which should be a property of any cipher!). That also leads you to a nice way to refactor your code: decrypting with a shift of 5 is the same thing as encrypting with a shift of -5. Try rewriting your code to look like this:

def encrypt(str)
 str.split('').map { |char| round(char, @shift) }.join
end
def decrypt(str)
 str.split('').map { |char| round(char, -@shift) }.join
end
private
def round(char, shift)
 # ...
end

Other than that, just a couple tips:

• Ways to compare ranges of characters: /[a-z]/i =~ chr or (?A..?Z) === chr
• What if @shift is greater than 26? You can use modulus % to account for shifts of any size

Answer 2

Max already pointed out some good ways to improve the actual character shifting (definitely use %!). I'll offer some low-level advice

• There's no reason to use the "bang" (!) version of map; just use the regular non-bang version

• Use String#chars instead of split

• You can since you pass the block parameter for map directly on to your round_* methods, you could just use map(&method(:round_down))...

• And you can in fact write it all as a chain

  def encrypt(str)
   str.chars.map(&method(:round_over)).join
  end
  

  and similar for decrypt

• round_over should be called round_up - it's normal English, and it's the more appropriate "opposite" of round_down. Eitherway, both those methods should probably be renamed shift_* or rotate_*, since they're not really rounding anything; they're shifting or rotating the characters

• Don't use match in a conditional when you just want a boolean; use Ruby's =~ operator to check a string against a regex: char =~ /[[:lower:]]/. match is better suited for when you want to pull matched parts out of a string; not when you just want a true/false

• You can use ?A and ?z etc. instead of "A".ord (just a handy shortcut/alternative syntax). Max also used this in a range expression.

• Again: Take Max's advice and rewrite the private methods. But eitherway, you should at least calculate char.ord +/- @shift once, and then do the if..elsif..else branching. Don't calculate it for every conditional and again for output.

Answer 3

In general, if you suspect that the problem you are tackling is rather common, Ruby probably has a method for it. In many cases you will be able to guess it's name - admittedly not in this case.

Ruby strings have an easily overlooked method named tr which replaces characters of a string with specified other characters. This specification is done on initialize, after which the actual en- and de-crypting is trivial.

class CaesarCipher
 ALFABET = ("a".."z").to_a 
 CHARS = "a-zA-Z"
 def initialize(shift)
 lkey = ALFABET.rotate(shift).join
 @key = lkey + lkey.upcase
 end
 def encrypt(str)
 str.tr(CHARS, @key)
 end
 def decrypt(str)
 str.tr(@key, CHARS)
 end
end

• RubberDuck: Great answer, but that kind of sucks the fun out of writing a cipher.