Caesar Cipher implementation - Odin Project

Question

I am going through the Odin Project Ruby section, and just finished implementing a Caesar cipher interpretation. Should I use classes, or more than one method to distribute the flow? Is there really a need for initialization in this?

def caesar_cipher(string, shifter)
 ciphered = []
 string.split("").each do |char|
 if char == " " then ciphered << char 
 next 
 end
 char = char.to_s.ord
 if ("a".ord.."z".ord).include? char
 if char + shifter < "z".ord
 ciphered.push((char + shifter).chr)
 else
 ciphered.push((shifter - ("z".ord - char) + "a".ord).chr)
 end 
 elsif ("A".ord.."Z".ord).include? char
 if char + shifter < "Z".ord
 ciphered.push((char + shifter).chr)
 else
 ciphered.push((shifter - ("Z".ord - char) + "A".ord).chr)
 end 
 else 
 next
 end
 end
 print ciphered.join + "\n"
end
#test: caesar_cipher("Hello friend my Name is Sam",10)

• Dan Kohn: Please see some of the other solutions already discussed: Learning Ruby Caesar Cipher, Caesar cipher in Ruby, Menu-based Caesar Cipher with encryption and decryption

Answer 1

The code can be simplified, but you're on the right track.

A function is sufficient for a simple string filter like this; a class would be overkill. I would rename the first parameter to plaintext and the result to ciphertext because that is the standard terminology.

The function should return the ciphertext rather than printing it. The caller should have the flexibility to print it, show it in a GUI, store it in a database, or whatever.

Instead of split("").each, you can iterate over the plaintext using plaintext#each_codepoint. You can also build the result by appending to a string rather than joining a list.

The if...elseif would be slightly better as a case expression. You don't need the special case for " "

def caesar_cipher(plaintext, shift)
 ciphertext = ''
 plaintext.each_codepoint do |asc|
 ciphertext << case asc
 when 'a'.ord..'z'.ord
 'a'.ord + (asc - 'a'.ord + shift) % 26
 when 'A'.ord..'Z'.ord
 'A'.ord + (asc - 'A'.ord + shift) % 26
 else
 asc
 end
 end
 ciphertext
end

• ppadru1: This is great, thank you! What does the each_codepoint method do?
• 200_success: String#each_codepoint is like each_char but yields the ord of each character.
• ppadru1: That is awesome, thanks! Also, I forgot the syntax of the case when statement that in each when you just specify the condition and don't need to repeat "asc in..". Very useful!
• Mark Thomas: This is a good answer. It might be possible to simplify further by using map to obviate the need for the intermediary ciphertext variable.