Monoalphatic and Polialphabetic cipher in Ruby

Question

This code encrypts a text with mono-alphabetic and poli-alphabetic substitutions ciphers.

For further info see:

• Mono-alphabetic/Caesar Cipher
• Poli-alphabetic cipher

require 'arrow_test'
ALPHABET = ("a".."z").to_a
# Moves a letter forward in the alphabet by
# the given key wrapping around.
#
# <code> 
# shift('a', 2) #=> 'c' 
# </code>
#
# <code> 
# shift('z', 2) #=> 'b' 
# </code>
def shift(letter, key)
 ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]
end
# Encrypts a text using by shifting all its letters by
# the given key. All characters that are not lower and alphabetic
# are deleted.
# 
# <code>
# monoalphabetic_cipher('abcd', 2) #=> 'cdef'
# </code>
# 
# <code>
# monoalphabetic_cipher('cdef', 2, decode=true) #=> 'abcd'
# </code>
def monoalphabetic_cipher(text, key, decode=false)
 text
 .chars
 .select { |char| ALPHABET.include?(char) }
 .map {|char| shift(char, (decode ? 26 - key : key) )}
 .join
end
# Given a word, returns the positions of its letters
# in the aplhabet.
#
# <code>
# word_to_alphabetic_positions('abcz') #=> [0, 1, 2, 25]
# </code>
def word_to_alphabetic_positions(word)
 word.chars.map {|char| ALPHABET.index(char) }
end
# Encrypts a text by <code>shift</code>ing each letter by
# the amount specified by the corresponding key number,
# wraps around over the key if the plain text is longer than it.
#
# <code>
# polialphabetic_cipher('abcd', [1,3,4,0]) #=> 'begd'
# </code>
#
# <code>
# polialphabetic_cipher('begd', [1,3,4,0], false, decode=true) #=> 'abcd'
# </code>
def polialphabetic_cipher(text, keys, keys_as_word=false, decode=false)
 keys = word_to_alphabetic_positions(keys) if keys_as_word
 text
 .chars
 .select { |char| ALPHABET.include?(char) }
 .each_with_index
 .map do |char, index|
 key = keys[index % keys.length]
 shift(char, (decode ? 26 - key : key))
 end
 .join
end
arrow_test

Answer 1

Extract constant expression to outside of loop

In this code, (decode ? 26 - key : key) is evaluated for each character in text:

 text
 .chars
 .select { |char| ALPHABET.include?(char) }
 .map {|char| shift(char, (decode ? 26 - key : key) )}
 .join

It would be better to move it outside the expression so that it's only evaluated once:

 shift_key = decode ? 26 - key : key
 text
 .chars
 .select { |char| ALPHABET.include?(char) }
 .map {|char| shift(char, shift_key)}
 .join

Minor formatting issues

The code formatting style is not consistent, for example compare these two lines:

 .select { |char| ALPHABET.include?(char) }
 .map {|char| shift(char, shift_key)}

In the first one, there's a space after { and before }, but in the second there isn't. Both ways seem fine to me, but it would be better to use one of these ways, consistently everywhere.

Similarly here, there's a space after [ but not before ]. For the sake of consistency, it would be better to remove the space after [ or add one before ].

 ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]

Overall

This looks nice, a pleasure to read, and I don't even know Ruby ;-)

• Caridorc: Extract constant expression to outside of loop works only in the mono-alphabetic cipher, in the poli-alphabetic one the key is not a loop invariant
• janos: Oops, my bad, dropped that point
• Caridorc: Ruby is a very nice language, I enjoy programming in it. You may be interested in Functional Programming in Ruby that webpage shortly explains the basics of effective ruby usage.