6
\$\begingroup\$

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
\$\endgroup\$

migrated from stackoverflow.com Jun 23 '14 at 17:36

This question came from our site for professional and enthusiast programmers.

5
\$\begingroup\$

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
\$\endgroup\$
4
\$\begingroup\$

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.

\$\endgroup\$
3
\$\begingroup\$

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
\$\endgroup\$
  • \$\begingroup\$ Great answer, but that kind of sucks the fun out of writing a cipher. \$\endgroup\$ – RubberDuck Jun 28 '14 at 10:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy