# Learning Ruby Caesar Cipher

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


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

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.

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

• Great answer, but that kind of sucks the fun out of writing a cipher. – RubberDuck Jun 28 '14 at 10:36