2

I have been working for a while on cipher program in python for an online course. I keep going back and forth between successes and set backs, and recently thought I had figured it out. That is, until I compared the output I was getting to what the course said I should actually be getting. When I input "The crow flies at midnight!" and a key of "boom", I should be getting back "Uvs osck rwse bh auebwsih!" but instead get back "Tvs dfci tzufg mu auebwsih!" I am at a loss for what my program is doing, and could use a second look at my program from someone. Unfortunately, I don't have a person in real life to go to lol. Any help is greatly appreciated.

alphabet = "abcdefghijklmnopqrstuvwxyz"
def alphabet_position(letter):
    lower_letter = letter.lower()   #Makes any input lowercase.
    return alphabet.index(lower_letter) #Returns the position of input as a number.

    def vigenere(text,key):
        m = len(key)
        newList = ""

        for i in range(len(text)):
            if text[i] in alphabet:
                text_position = alphabet_position(text[i])
                key_position =  alphabet_position(key[i % m])
                value = (text_position + key_position) % 26
                newList += alphabet[value]
            else:
                newList += text[i]
        return newList


    print (vigenere("The crow flies at midnight!", "boom"))

    # Should print out Uvs osck rmwse bh auebwsih!
    # Actually prints out Tvs dfci tzufg mu auebwsih!
Brian Burns
  • 113
  • 2
  • 13

3 Answers3

1

In your vigenere function, convert set text = text.lower() .
To find such problems just follow one letter and see what happens, it was very easy to see that it doesn't work because 'T' is not in the alphabet but 't' is so you should convert the text to lower case.

1

It looks like the problem is that you didn't remind to handle the spaces. The "m" of "boom" should be used to encrypt the "c" of "crow", not the space between "The" and "crow"

EsotericVoid
  • 2,306
  • 2
  • 16
  • 25
  • Oh, how would I go about making the program mindful of the spaces? – Brian Burns Jun 25 '17 at 23:20
  • 1
    @Harindu Dilshan summed it pretty well, on another note you should consider studying a bit the built-ins in python. You could save indentation space by using the enumerate() function – EsotericVoid Jun 25 '17 at 23:26
  • Thank you, I will do that. The course is only teaching the very basics, so built-ins aren't explained much yet. – Brian Burns Jun 25 '17 at 23:29
  • I quickly looked up the enumerate function. Are you saying in this case I should have used it instead of "for i in range(len(text))"? – Brian Burns Jun 25 '17 at 23:43
  • 1
    Yes, something like 'for i, c in enumerate(text):' is a bit more pythonic and makes your code more readable to other programmers (I'll correct myself, it's not about indentation in this case) – EsotericVoid Jun 25 '17 at 23:52
  • Oh ok, thank you! I'm going to tinker the code to see how to to insert that into my code. Any other specifics you think I should look into to make it more pythonic? – Brian Burns Jun 25 '17 at 23:53
1

Ok.The problem was the expected cipher skipped non-alphabetical characters and continued on the next letter with the same key.But in your implementaion you skipped the key too.

The crow

boo mboo // expected

boo boom // your version

So here is the corrected code:

alphabet = "abcdefghijklmnopqrstuvwxyz"
def alphabet_position(letter):
lower_letter = letter.lower()   #Makes any input lowercase.
return alphabet.index(lower_letter)  #Returns the position of input as a number.

def vigenere(text,key):
    text_lower = text.lower()
    m = len(key)
    newList = ""
    c = 0
    for i in range(len(text)):
        if text_lower[i] in alphabet:
            text_position = alphabet_position(text[i])
            key_position =  alphabet_position(key[c % m])
            value = (text_position + key_position) % 26
            if text[i].isupper():
              newList += alphabet[value].upper()
            else:  
              newList += alphabet[value]
            c += 1
        else:
            newList += text[i]
            
    return newList


 print (vigenere("The crow flies at midnight!", "boom"))
 # Should print out Uvs osck rmwse bh auebwsih!
 # Actually prints out Tvs dfci tzufg mu auebwsih!
Community
  • 1
  • 1
  • Thank you! It worked perfect. I am currently studying it to actually understand the difference it is making to my code. – Brian Burns Jun 25 '17 at 23:36
  • It uses a new variable 'c' for the position of key letter. So you can choose to skip or not when encountering a non alphabetical character. – Harindu Dilshan Jun 26 '17 at 02:57
  • Thank you! I was just looking at it and looking up some more technical uses of modulo in programming to better understand them – Brian Burns Jun 26 '17 at 02:59