HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Monoalphatic and Polialphabetic cipher in Ruby

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
monoalphaticpolialphabeticrubyandcipher

Problem

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.
#
#  
# shift('a', 2) #=> 'c' 
# 
#
#  
# shift('z', 2) #=> 'b' 
# 

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.
# 
# 
# monoalphabetic_cipher('abcd', 2) #=> 'cdef'
# 
# 
# 
# monoalphabetic_cipher('cdef', 2, decode=true) #=> 'abcd'
# 

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.
#
# 
# word_to_alphabetic_positions('abcz') #=> [0, 1, 2, 25]
# 

def word_to_alphabetic_positions(word)
  word.chars.map {|char| ALPHABET.index(char) }
end

# Encrypts a text by shifting each letter by
# the amount specified by the corresponding key number,
# wraps around over the key if the plain text is longer than it.
#
# 
# polialphabetic_cipher('abcd', [1,3,4,0]) #=> 'begd'
# 
#
# 
# polialphabetic_cipher('begd', [1,3,4,0], false, decode=true) #=> 'abcd'
# 

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

Solution

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

Code Snippets

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

  text
      .chars
      .select { |char| ALPHABET.include?(char) }
      .map {|char| shift(char, shift_key)}
      .join
.select { |char| ALPHABET.include?(char) }
      .map {|char| shift(char, shift_key)}
ALPHABET[ (ALPHABET.index(letter) + key) % ALPHABET.length]

Context

StackExchange Code Review Q#96324, answer score: 4

Revisions (0)

No revisions yet.