HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Defining functions in Vigenere cipher

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionsvigeneredefiningcipher

Problem

I'm trying to learn the process of defining/calling a (somewhat) complex function.

I understand the main idea, and can do it on a small (simple) scale.

A simple example:

def multiply(x, y):
     return x*y

numb1 = 2
numb2 = 3

print(multiply(numb1, numb2))


Now what I'm trying to do is clean up my Vigenere cipher by defining the functions, and what's stumping me is what parameters to use and why. I don't want to change the (possibly poorly-written) code, however I just want to see how it would look after making use of defined functions.

message = input("Enter a message to encrypt:\n").upper().replace(" ", "")
print("Enter your encryption key ("+str(len(message)),"or less letters.): ")
key = input().upper().replace(" ", "") 
div_times = int(int(len(message))/int(len(key)))
remainder = int(len(message))%int(len(key))
key_ring = (((key)*(div_times+1))[:-(len(key)-remainder)])
alph = 26 

print("-------\n"
      "Message:  ",message,"\n"
      "Key:      ",key_ring,
      "\n-------")
mvalues = [0]*len(message)
kvalues = [0]*len(key_ring)
m_position = 0
k_position = 0

for letter in message:
    mvalues[m_position] = ord(letter)
    m_position += 1

for key in key_ring:
    num = alph - (int(ord("Z")) - int(ord(key)))
    kvalues[k_position] = num - 1
    k_position += 1

m_position = 0
k_position = 0
print("\nEncrypted message: ", end="")
for character in message:
    newletter_v = (mvalues[m_position] + kvalues[k_position])
    if newletter_v > ord("Z"):
        newletter_v -= 26
    elif newletter_v < ord("A"):
        newletter_v += 26
    print(chr(newletter_v)+"", end="")
    m_position += 1
    k_position += 1

Solution

I think you're making this a lot more complicated than it needs to be. Forgive my rusty python, but you should be able to do something like

def viginere_encrypt(message, key):
    length = len(message)
    mnum = [0]*length
    for i in range(0, length):
        mnum[i] = letter_to_number(message[i])
    knum = [0]*len(key)
    for i in range(0, len(key)):
        knum[i] = letter_to_number(key[i]
    result = [0]*length
    for i in range(0, length):
        result[i] = number_to_letter(mnum[i] + knum[i % len(key])
    return result

def letter_to_num(letter):
    return ord(letter) - ord("a")

def number_to_letter(number):
    return chr(number%alpha + ord("a"))


Using list comprehensions, this can be shortened to

def viginere_encrypt(message, key):
    mnum = [letter_to_number(x) for x in message]
    knum = [letter_to_number(x) for x in key]
    return [number_to_letter(mnum[i] + knum[i%len(key)])for i in range(0, len(message)]


or even further to

def viginere_cipher(message, key):
    return [number_to_letter(letter_to_number(message[i]) + letter_to_number(key[i % len(key)])) for i in range(0, len(message)])


though that's probably too mushed together by now.

Also, I don't know python too well; there's probably a better way to convert between "a-z" and 0-25.

Anyways, functions are a Very Good Idea. Output can then be separated from business logic, so if, say, you need to use the cipher elsewhere in your code, you're not tied to the terminal. It also allows for much easier testing.

Code Snippets

def viginere_encrypt(message, key):
    length = len(message)
    mnum = [0]*length
    for i in range(0, length):
        mnum[i] = letter_to_number(message[i])
    knum = [0]*len(key)
    for i in range(0, len(key)):
        knum[i] = letter_to_number(key[i]
    result = [0]*length
    for i in range(0, length):
        result[i] = number_to_letter(mnum[i] + knum[i % len(key])
    return result

def letter_to_num(letter):
    return ord(letter) - ord("a")

def number_to_letter(number):
    return chr(number%alpha + ord("a"))
def viginere_encrypt(message, key):
    mnum = [letter_to_number(x) for x in message]
    knum = [letter_to_number(x) for x in key]
    return [number_to_letter(mnum[i] + knum[i%len(key)])for i in range(0, len(message)]
def viginere_cipher(message, key):
    return [number_to_letter(letter_to_number(message[i]) + letter_to_number(key[i % len(key)])) for i in range(0, len(message)])

Context

StackExchange Code Review Q#67502, answer score: 3

Revisions (0)

No revisions yet.