Reputation: 101
I've been trying to work the following problem out and ran into the error. The point of the problem is to use a given key sequence to encrypt a string. For example, when given "cat" and [1,2,3] the result should be "dcw" Any suggestions? the error was the following
def vigenere_cipher(string, key_sequence)
keyIndex=0
string=string.each_char.map do |c|
c=c.shift!(c,keyIndex)
keyIndex+=1
if keyIndex=key_sequence.length
keyIndex=0
end
end
return string
end
def shift!(c,keyIndex)
alphabet = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"]
inititalLetterIndex=alphabet.index(c)
finalLetterIndex=alphabet[inititalLetterIndex+keyIndex]
return alphabet[finalLetterIndex]
end
vigenere_cipher("cat", [1,2,3])
# private method `shift!' called for "c":String (NoMethodError)
Upvotes: 2
Views: 3487
Reputation: 54263
cipher.rb:4:in `block in vigenere_cipher': private method `shift!' called for "c":String (NoMethodError)
shift!
isn't defined in String class, but at the top level.
So replace c=c.shift!(c,keyIndex)
by c=shift!(c,keyIndex)
cipher.rb:17:in `[]': no implicit conversion of String into Integer (TypeError)
Line 16 defines :
finalLetterIndex=alphabet[inititalLetterIndex+keyIndex]
alphabet contains letters as Strings, so finalLetterIndex
isn't an index (Numeric), but a String.
On line 17, you try to use this String as an index.
Replace line 16 with :
finalLetterIndex=inititalLetterIndex+keyIndex
Your script doesn't raise any exception anymore. It also doesn't display anything, so add a puts to the last line :
puts vigenere_cipher("cat", [1,2,3]).inspect
It returns :
[0, 0, 0]
keyIndex
seems to be stuck at 0. Why?
Look at line 6 :
if keyIndex=key_sequence.length
It doesn't test an equality, it assigns keyIndex
to key_sequence.length
.
Since any number is truthy in Ruby, it executes the code inside the if statement. Replace with
if keyIndex==key_sequence.length
Your code returns [nil, nil, 0]
. Why?
string
is defined as the result of map
. map
returns an Array, in which each element is the result of the last executed command inside the block : in this case, the if
statement.
if
returns nil
when the condition isn't satisfied, and returns the last executed command otherwise. In this case 0
.
Add c
at the last line of your map
block.
Your code now returns ["c", "b", "v"]
. Why?
You only shift by shiftIndex
, not by the amount defined in key_sequence
Array. Replace
c=shift!(c,keyIndex)
with
c=shift!(c,key_sequence[keyIndex])
Your code returns ["d", "c", "w"]
. Almost there!
Ruby is a dynamic language. You're free to overwrite the String string
with an Array, but it will confuse others and your future self.
Use array
or letters
instead of string
, and return letters.join
Your script now returns "dcw"
.
It should look like :
def vigenere_cipher(string, key_sequence)
keyIndex=0
letters=string.each_char.map do |c|
c=shift!(c,key_sequence[keyIndex])
keyIndex+=1
if keyIndex==key_sequence.length
keyIndex=0
end
c
end
return letters.join
end
def shift!(c,keyIndex)
alphabet = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"]
inititalLetterIndex=alphabet.index(c)
finalLetterIndex=inititalLetterIndex+keyIndex
return alphabet[finalLetterIndex]
end
vigenere_cipher("Hello", [1,2,3])
raises
cipher.rb:17:in 'shift!': undefined method '+' for nil:NilClass (NoMethodError)
.
Well, 'H' isn't found in your alphabet. Use downcase :
array=string.downcase.each_char.map do |c|
vigenere_cipher("Hello World", [1,2,3])
doesn't work either, because of the space. Delete anything that isn't a letter :
array=string.downcase.delete('^a-z').each_char.map do |c|
vigenere_cipher("zzz", [1,2,3])
returns an empty String, because there's no letter after z
.
Use modulo 26 :
return alphabet[finalLetterIndex%26]
Remove typos, don't use camelCase for variables, remove unnecessary return
and you get :
def vigenere_cipher(string, key_sequence)
key_index = 0
letters = string.downcase.delete('^a-z').each_char.map do |c|
c = shift(c, key_sequence[key_index])
key_index = (key_index + 1) % key_sequence.length
c
end
letters.join
end
def shift(c, key_index)
alphabet = ('a'..'z').to_a
initial_letter_index = alphabet.index(c)
final_letter_index = initial_letter_index + key_index
alphabet[final_letter_index % 26]
end
Using each_char
, zip
and cycle
, I'd rewrite the whole code this way :
class Integer
# 0 => 'a', 1 => 'b', ..., 25 => 'z', 26 => 'a'
def to_letter
('a'.ord + self % 26).chr
end
end
class String
# 'A' => '0', 'a' => 0, ..., 'z' => 25
def to_code
self.downcase.ord - 'a'.ord
end
end
def vigenere_cipher(string, key)
short_string = string.delete('^A-Za-z')
short_string.each_char.zip(key.cycle).map do |char, shift|
(char.to_code + shift).to_letter
end.join
end
Wikipedia article uses a String as key :
def vigenere_cipher(string, key)
short_string = string.delete('^A-Za-z')
short_string.each_char.zip(key.each_char.cycle).map do |char, shift|
(char.to_code + shift.to_code).to_letter
end.join
end
vigenere_cipher('Attack at dawn!', 'LEMON').upcase # => "LXFOPVEFRNHR"
You should also be able to decrypt the message :
def vigenere_cipher(string, key, decrypt = false)
short_string = string.delete('^A-Za-z')
short_string.each_char.zip(key.each_char.cycle).map do |char, shift|
(char.to_code + shift.to_code * (decrypt ? -1 : 1)).to_letter
end.join
end
vigenere_cipher("LXFOPVEFRNHR", 'LEMON', :decrypt) #=> "attackatdawn"
Well, that was longer than expected! :D
Upvotes: 1
Reputation: 4920
If you want to call you method shift!
on a string, you will have to define it on String
class.
class String
def shift!(keyIndex)
# you can access `c` using `self` here
...
end
end
Then you can call it as c.shift!(keyIndex)
(Note the arguments are different).
Upvotes: 1
Reputation: 2183
You are trying to call shift!
on string object that does not define on String
Class, instead you defined on main object. You can call it like shift!(c,keyIndex)
instead of c.shift!(c,keyIndex)
Upvotes: 3