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

Caesar cipher in Ruby

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

Problem

I must implement a simple Caesar cipher class. Initializer takes two parameters - number of positions to shift the alphabet (integer) and the alphabet used (as string, not necessarily the standard English alphabet). The class must have two instance methods encrypt and decrypt, which take a string and do the respective action based on the parameters of the initializer. These are my rspec tests:

describe "Caesar" do

  latin_encrypter = Caesar.new 4
  dna_encrypter = Caesar.new 3, 'ACTG'

  it 'encrypts correctly' do
    expect(latin_encrypter.encrypt 'hello').to eq "lipps"
  end

  it 'encrypts correctly using DNA alphabet' do
    expect(dna_encrypter.encrypt 'ACCTGA').to eq "GAACTG"
  end

  it 'decrypts correctly' do
    expect(latin_encrypter.decrypt 'lipps').to eq 'hello'
  end

  it 'decrypts correctly using DNA alphabet' do
    expect(dna_encrypter.decrypt 'GAACTG').to eq 'ACCTGA'
  end
end


and this is my implementation:

class Caesar
  def initialize(shift, alphabet = 'abcdefghijklmnopqrstuvwxyz')
    @shift = shift % alphabet.size
    @alphabet = alphabet
  end

  def encrypt(string)
    string.chars.map { |char| @alphabet[@alphabet.index(char) + @shift - @alphabet.size] }.join
  end

  def decrypt(string)
    string.chars.map { |char| @alphabet[@alphabet.index(char) - @shift] }.join
  end
end


Is this an optimal algorithm for encryption/decryption or is there a better one (perhaps using gsub)?

Solution

The only (conceptual) problem I see in your code is that both encrypt/decrypt perform a O(n) operation (detect) for every character (so at then end it's a O(n*m) algorithm), that's unnecessarily inefficient. Build a hash object to use as an indexer:

class Caesar
  def initialize(shift, alphabet = ('a'..'z').to_a.join)
    @shift = shift
    @alphabet = alphabet
    @indexes = alphabet.chars.map.with_index.to_h
  end

  def encrypt(string)
    string.chars.map { |c| @alphabet[(@indexes[c] + @shift) % @alphabet.size] }.join
  end

  def decrypt(string)
    string.chars.map { |c| @alphabet[(@indexes[c] - @shift) % @alphabet.size] }.join
  end
end


You can also build encrypter/decrypter hash tables at initialization:

class Caesar
  def initialize(shift, alphabet = ('a'..'z').to_a.join)
    chars = alphabet.chars.to_a
    @encrypter = chars.zip(chars.rotate(shift)).to_h
    @decrypter = @encrypter.invert
  end

  def encrypt(string)
    @encrypter.values_at(*string.chars).join
  end

  def decrypt(string)
    @decrypter.values_at(*string.chars).join
  end
end

Code Snippets

class Caesar
  def initialize(shift, alphabet = ('a'..'z').to_a.join)
    @shift = shift
    @alphabet = alphabet
    @indexes = alphabet.chars.map.with_index.to_h
  end

  def encrypt(string)
    string.chars.map { |c| @alphabet[(@indexes[c] + @shift) % @alphabet.size] }.join
  end

  def decrypt(string)
    string.chars.map { |c| @alphabet[(@indexes[c] - @shift) % @alphabet.size] }.join
  end
end
class Caesar
  def initialize(shift, alphabet = ('a'..'z').to_a.join)
    chars = alphabet.chars.to_a
    @encrypter = chars.zip(chars.rotate(shift)).to_h
    @decrypter = @encrypter.invert
  end

  def encrypt(string)
    @encrypter.values_at(*string.chars).join
  end

  def decrypt(string)
    @decrypter.values_at(*string.chars).join
  end
end

Context

StackExchange Code Review Q#35317, answer score: 4

Revisions (0)

No revisions yet.