CODEWITHSUNDEEP
pythonpython-2.7lambdavigenere
user6327497
user6327497

Reputation:

Simplifying Vigenere cipher program in Python

I have the program below, which is passed on to another function which simply prints out the original and encrypted messages. I want to know how I can simplify this program, specifically the "match = zip" and "change = (reduce(lambda" lines. If possible to do this without using lambda, how can I?

from itertools import cycle

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

def vigenereencrypt(message,keyword):
    output = ""    
    match = zip(message.lower(),cycle(keyword.lower()))
    for i in match:
        change = (reduce(lambda x, y: alphabet.index(x) + alphabet.index(y), i)) % 26
        output = output + alphabet[change]
    return output.lower()

Upvotes: 0

Views: 523

Answers (3)

Brendan Long
Brendan Long

Reputation: 54292

Starting with what R Nar said:

def vigenereencrypt(message,keyword):
    output = ""
    for x, y in zip(message.lower(), cycle(keyword.lower())):
        change = (alphabet.index(x) + alphabet.index(y)) % 26
        output = output + alphabet[change]
    return output.lower()

We can be more efficient by using a list and then joining it, instead of adding to a string, and also by noticing that the output is already lowercase:

def vigenereencrypt(message,keyword):
    output = []
    for x, y in zip(message.lower(), cycle(keyword.lower())):
        change = (alphabet.index(x) + alphabet.index(y)) % 26
        output.append(alphabet[change])
    return "".join(output)

Then we can reduce the body of the loop to one line..

def vigenereencrypt(message,keyword):
    output = []
    for x, y in zip(message.lower(), cycle(keyword.lower())):
        output.append(alphabet[(alphabet.index(x) + alphabet.index(y)) % 26])
    return "".join(output)

... so we can turn it into a list comprehension:

def vigenereencrypt(message,keyword):
    output = (
        alphabet[(alphabet.index(x) + alphabet.index(y)) % 26]
        for x, y in zip(message.lower(), cycle(keyword.lower()))
    )
    return "".join(output)

I feel like there's something we could do with map(alphabet.index, ...) but I can't think of a way that's any better than the list comprehension.

Upvotes: 2

kpie
kpie

Reputation: 11110

you could do it with a bunch of indexing instead of zip...

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"]
alphaSort = {k:n for n,k in enumerate(alphabet)}
alphaDex = {n:k for n,k in enumerate(alphabet)}

def vigenereencrypt(message,keyword):
    output = ""    
    #match = zip(message.lower(),cycle(keyword.lower()))      # zip(a,cycle(b)) Creates [(a[n],b[n%len(b)]) for k in range(len(a)) ] 
    op = ""                                                  # So lets start with  for k in range(len(a))
    for k in range(len(message)):
        op += alphaDex[(alphaSort[message.lower()[k]]+alphaSort[keyword.lower()[k%len(keyword)]])%len(alphabet)]
    return(op)

Upvotes: 0

R Nar
R Nar

Reputation: 5515

Two things:

  1. You dont need to have a local variable match, just loop zip
  2. Your can split up your two indices x and y in your for loop definition rather than using reduce; reduce is normally used for larger iterables and since you only have 2 items in i, it's adding unnecessary complexity.

ie, you can change your for loop definition to:

for x, y in zip(...):

and your definition of change to:

change = (alphabet.index(x) + alphabet.index(y)) % 26

Upvotes: 2

Related Questions