I was trying to refactor the following code for Caesar cipher, but it feels overly complicated at the end of the refactoring. Could someone else take a look at it and let me know if I am just overdoing/making it complicated for someone trying to read the code?

My highest priority is to write readable code, so any pointers in that regard would be welcome.

module CASE_WRAP
def cyclic_next!
if self == 'z'
replace 'a'
return
end

if self == 'Z'
replace 'A'
return
end

self.next!
end

def shift(number)
number.times { self.cyclic_next! }
self
end
end

class CaesarCipher
def self.cipher(message, shift)
CaesarCipher.new(shift: shift)
.encrypt message
end

def initialize(shift: 0)
@shift = shift
end

def encrypt(message)
message.gsub(/\w/) do |character|
character.extend(CASE_WRAP)
.shift @shift
end
end
end


Find tests for this in my repo.

All the caesar cypher does is rotate an alphabet by n places. If we let the alphabet be an Array, we can use Array#rotate to do the rotating for us. To substitute single characters, there is String#tr. Its name and usage are a little odd, but you'll quickly get used to it.

Basically, it maps characters in a String to characters in another String by their index. So "abc".tr("abc", "123") maps "a" to "1", "b" to "2" and "c" to "3".It expands ranges such as "a-c" first, so you can can write "abc".tr("abc", "123") as "abc".tr("a-c","1-3").

Though the interface is different, here's a simple implementation of the Caesar cypher.

module Caesar
def self.encrypt(message, shift = 3)
message.tr("a-zA-Z", rotated_alphabet(shift))
end

def self.decrypt(message, shift = 3)
encrypt(message, 26 - shift)
end

def self.rotated_alphabet(shift)
lower = "a".upto("z").to_a.rotate(shift)
upper = "A".upto("Z").to_a.rotate(shift)

lower.concat(upper).join
end
end

Caesar.decrypt Caesar.encrypt("hi")
# => "hi"


For efficiency, you could change how you calculate the rotated alphabets. Here's an efficient, but not exactly a readable implementation.

  def self.rotated_alphabet(shift)
lower = (97 + shift).chr # 97 is the ASCII code for "a"
upper = (65 + shift).chr # 65 is the ASCII code for "A"

"#{lower}-za-#{lower}#{upper}-ZA-#{upper}"
end

• Thanks for the pointer to tr, I knew the method before; but couldn't see it was ideal for this problem. I have now updated my repo with the refactoring based on your suggestion: github.com/JikkuJose/katas/blob/master/caesar_cipher/01/cc.rb Feb 11, 2015 at 18:33