Asterus
Asterus

Reputation: 1

Python vigenere cipher going further than needed?

I am attempting to create the vigenere cipher in python and there seems to be a problem. Here is my encryption code:

def encryption():
    plaintext=input("Please enter the message you wish to encode.")
    #This allows the user to enter the message they wish to encrypt.
    keyword=input("Please enter your keyword, preferably shorter than the plaintext.")
    #This allows the user to enter a keyword.
    encoded=""
    #This creates a string for the user to input the encrypted message to.
    while len(keyword)<len(plaintext):
        #This begins a while loop based on the fact that the length of the keyword is shorter than the length of the plaintext.
        keyword+=keyword
        #This repeats the keyword.
        if len(keyword)>len(plaintext):
            #This sees if the string length of the keyword is now longer than the string length of the plaintext.
            newkey=keyword[:len(plaintext)]
            #This cuts the string length of the keyword
    for c in range(len(plaintext)):
        char=ord(plaintext[c])
        temp=ord(keyword[c])
        newchar=char+temp
        if newchar>ord("Z"):
            newchar-=26
        newnewchar=chr(newchar)
        encoded+=newnewchar
    print(encoded)

I cannot seem to find the problem with it, however when I enter the plaintext "hello" and the keyword "hi" it come up with the following symbols: ¶´º»½. I think the addition in the for loop may be going too far.

Upvotes: 0

Views: 10999

Answers (3)

Yann Vernier
Yann Vernier

Reputation: 15887

Indeed the addition goes too far, because ord uses ASCII (where A is 65), while de Vigenère had A in the first position. You could subtract ord('A'). The code also assumes all characters are capital letters. Here's a variation that uses a few of Python's library functions to perform the task.

import string, itertools
def encrypt(text, key='N'):     # default to rot13 (:
    '''Performs a Vigenere cipher of given plaintext and key'''
    result=[]
    key=key.upper()
    for plain,shift in itertools.izip(text,itertools.cycle(key)):
        shiftindex=string.ascii_uppercase.index(shift)
        shiftto=(string.ascii_uppercase[shiftindex:] +
                 string.ascii_uppercase[:shiftindex])
        trans=string.maketrans(string.ascii_letters,
                               shiftto.lower()+shiftto)
        result.append(plain.translate(trans))
    return ''.join(result)

A more complete variant might only consume key for letters, but this will be fine if your strings only contain letters. The reason I stuck to the ASCII letters is because locale specific alphabets might not have the intended ordering nor a matching set of upper and lower case (for instance, german ß). It's also entirely possible to translate the key into a list of translation tables only once.

Upvotes: 0

ThinkChaos
ThinkChaos

Reputation: 1853

I won't just solve the bug for you as there are many other things to improve here !
Put space around operators: a=b should be a = b, same with +, -, etc.

I find it better to use function params than input. You can always have a second function to get input and encrypt the input:

def encryption(plaintext, keyword):
    pass  # (do nothing)

I'll let you write the auxiliary function.

I, and most, usually put comments on the line above the corresponding code. Also, no need to write This every time, and imperative is generally preferred.

Now let's have a look at your while loop. The condition is len(keyword) < len(plaintext), inside you check len(keyword) > len(plaintext). When can that happen ? Only during the last iteration. So move the code out of the loop.
Also, what you do inside the if doesn't need an if:
any_string[:len(any_string) + n] == any_string (n being a positive int).
Plus you never use newkey!

So we can simplify the loop to:

# Begin a while loop based on the fact that the length of the keyword is
# shorter than the length of the plaintext.
while len(keyword) < len(plaintext):
    # Repeat the keyword.
    keyword += keyword

# Cut the string length of the keyword
keyword = keyword[:len(plaintext)]

Which is equivalent to:

# Do the operation once
txt_len = len(plaintext)
# Get a string just a bit longer than plaintext
# Otherwise we would have issues when plaintext is not a multiple
# of keyword
keyword *= txt_len // len(keyword) + 1
# Chop of the extra part
keyword = keyword[:txt_len]

Note that both will fail when len(keyword) == 0.

Now to the for loop:
You could use zip, as polku showed you, but I'll assume it's too complicated for now and keep the range. You could also use the alphabet, but plain arithmetic can do the trick:
alphabet.index(x) == ord(x) - ord('a'), so in your code:

char = ord(plaintext[c]) - ord('a')
temp = ord(keyword[c]) - ord('a')
newchar = char + temp
# Or simply:
newchar = ord(plaintext[c]) + ord(keyword[c]) - 194  # 2 * ord('a')

If we ignore capital letters, we can safely substitute

if newchar > 25:
    newchar -= 25
# With
newchar %= 25

Finally: alphabet[i] == ord(i + ord('a')).

Here's all this put together:

def encryption(plaintext, keyword):
    # Do the operation once
    txt_len = len(plaintext)

    # Get a string just a bit longer than plaintext
    # Otherwise we would have issues when plaintext is not a multiple
    # of keyword
    keyword *= txt_len // len(keyword) + 1  # // is floor division
    # Chop of the extra characters (if there are any)
    keyword = keyword[:txt_len]

    # Create a string to store the encrypted message
    encoded = ""

    # Now you should change this to work with capital letters
    for c in range(txt_len):
        # 194 = 2 * ord('a')
        newchar = ord(plaintext[c]) + ord(keyword[c]) - 194
        newchar %= 25
        encoded += chr(newchar + 97)  # 97 = ord('a')

    return encoded


def encrypt_input():
    # This function should use input to get plaintext and keyword
    # Then use encryption with those strings and print the result
    pass

Upvotes: 0

polku
polku

Reputation: 1590

You need to understand the ord() function, chr() is the inverse of ord()

 for i in range(300):
     print(str(i) + ' ' + chr(i))

If you don't use Unicode characters, you can use an alphabet string

alphabet = 'abcdefghijklmnopqrstuvwxyz'
for p,k in zip(plaintext,keyword): # you can directly iterate strings
    char = alphabet.index(p)
    temp = alphabet.index(k)
    newchar = char + temp
    if newchar > 25:
        newchar -= 25
    newchar = alphabet[newchar]

Upvotes: 1

Related Questions