6
\$\begingroup\$

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

For further info see:

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

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 ;-)

\$\endgroup\$
3
  • \$\begingroup\$ 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 \$\endgroup\$
    – Caridorc
    Sep 5 '15 at 21:27
  • \$\begingroup\$ Oops, my bad, dropped that point \$\endgroup\$
    – janos
    Sep 5 '15 at 21:28
  • \$\begingroup\$ 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. \$\endgroup\$
    – Caridorc
    Sep 5 '15 at 21:42

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.