Jonathan Math
Jonathan Math

Reputation: 101

private method called noMethodError ruby

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

Answers (3)

Eric Duminil
Eric Duminil

Reputation: 54263

Step 1

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)

Step 2

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

Step 3

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]

Step 4

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

Step 5

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.

Step 6

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

Step 7

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

Step 8

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|

Step 9

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|

Step 10

vigenere_cipher("zzz", [1,2,3])

returns an empty String, because there's no letter after z.

Use modulo 26 :

return alphabet[finalLetterIndex%26]

Step 11

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

Step 12

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

Step 13

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"

Step 14

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

Jagdeep Singh
Jagdeep Singh

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

Engr. Hasanuzzaman Sumon
Engr. Hasanuzzaman Sumon

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

Related Questions