Search code examples
pythonencryptionalphabetvigenere

Python Vigenere working, but I can't account for the spaces and non alphabetical characters using functions


I am currently working on a cipher program for a beginners python course. We first were told to create a function that would return the position of a given letter, using a string of the alphabet as a reference (that is my alphabet_position function.) Next, we were told to make a function that would allow for a single letter to be rotated by a chosen number(that is my rotate_character function). Third, we were tasked with creating a basic caesar cipher using the previous two functions. All of those I was able to make work as demonstrated by my code below.

The vigenere, however, is proving much more difficult. I was actually able to find a snippet of code that I was able to modify with my first function (alphabet_position) to make work if only alphabetic characters are used, but as soon as i enter any non alphabetical character (such as ! or ?) I get a return of ValueError: Substring Not found. When the program encounters these non alphabetical characters, the key is supposed to skip over them and carry the Nth character of the key to the next alphabetical character.

I am guessing the answer lies in somehow incorporating my rotate_character function into my Encrypt function, but I am unsure how to do this, as the rotate_character function expects a alphabetical character, and the vigenere function turns that parameter into an int before running it through.

Any advice? And as I am a new programmer, I will gladly take any other helpful criticism on my coding practices you may want to instill!`

> #Create function alphabet_position(letter) to turn letter into number
> #such as a=0 or e=4, using lowercase to make sure case doesnt matter. 
  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 rotate_character(char, rot):
>     if char.isalpha():
>         a = alphabet_position(char);
>         a = (a + rot) % (int(len(alphabet)));            #needs modulo
>         a = (alphabet[a]);
>         if char.isupper():
>             a = a.title()
>         return a
>     else:
>        return char
> 
> def caesar(text, rot):
>     list1 = ""
>     for char in text:
>         list1 += rotate_character(char, rot)
>     return list1
> 
> def vigenere(text,key):       
    m = len(key)
> 
>   newList = ""
> 
>   for i in range(len(text)):      
        text_position = alphabet_position(text[i])      
        key_position =  alphabet_position(key[i % m])       
        value = (text_position + key_position) % 26         
        newList += alphabet[value]      
    return newList
> 
> def main():
>     x = input("Type a message: ")
>     y = input("Rotate by: ")
>     result = vigenere(x, y)
>     print (result)
> 
> if __name__ == '__main__':   
      main()

Solution

  • No, you don't need the rotate function anymore. You just need to directly add any character that is not in the alphabet to newlist and then skip the encryption part.

    Now a sub-optimal way of doing this is to use if ... in ...:

    if text[i] in alphabet:
        # do your thing
    else:
        newList += text[i]
    

    Of course more optimal is to only go through the alphabet once and use a variable:

    pt_c = text[i]
    pt_i = alphabet.find(pt_c) # returns -1 instead of an error when not found
    if pt_i == -1:
        newList += pt_c
    else:
        newList += pt_c
        # do your thing *with the given index*
    

    This won't make any difference in the runtime for a Vigenère cipher of course. But it shows you how to think of efficient programming for later: there is no need to search twice.

    You could also continue the loop instead of having an else statement:

    pt_c = text[i]
    pt_i = alphabet.find(pt_c) # returns -1 instead of an error when not found
    if pt_i == -1:
        continue
    # do your thing with the given index
    

    this will make the indentation depth of your loop (the amount of scopes) less, with the unfortunate side effect of making your loop more complex (creating a local exit point).