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

Vigenère cipher in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pythonvigenèrecipher

Problem

Over the last year I have been programming on my own, mostly in Python. I haven't had any formal education, though I have followed some online instructions, and have been making things that interest me.

One of those things is my Vigenère encrypter. I've made a few versions of it, and I would appreciate feedback on my latest version. Such as, does it follow somewhat of a good design pattern for these things? How would it perform under heavy load?

If I can provide any additional information, please ask and I won't hesitate to do so.

This is my code:

```
from os import system

#characters to numbers table
dictDown = {'|':0,'a':1,'b':2,'c':3,'d':4,'e':5,'f':6,'g':7,'h':8,
'i':9,'j':10,'k':11,'l':12,'m':13,'n':14,'o':15,'p':16,
'q':17,'r':18,'s':19,'t':20,'u':21,'v':22,'w':23,'x':24,
'y':25,'z':26,'.':27,',':28,"'":29,'!':30,':':31,'1':32,
'2':33,'3':34,'4':35,'5':36,'6':37,'7':38,'8':39,'9':40,
'0':41,'=':42,'-':43,'"':44,'+':45,'[':46,'_':47,'(':48,
')':49,' ':50,'?':51,'\n':52,'@':53,'#':54,'/':55,'%':56,
'^':57,'*':58,']':59,'{':60,'}':61,'A':62,'B':63,'C':64,
'D':65,'E':66,'F':67,'G':68,'H':69,'I':70,'J':71,'K':72,
'L':73,'M':74,'N':75,'O':76,'P':77,'Q':78,'R':79,'S':80,
'T':81,'U':82,'V':83,'W':84,'X':85,'Y':86,'Z':87,'':89,';':90}

#numbers to characters table
dictUp = {0:'|',1:'a',2:'b',3:'c',4:'d',5:'e',6:'f',7:'g',8:'h',
9:'i',10:'j',11:'k',12:'l',13:'m',14:'n',15:'o',16:'p',
17:'q',18:'r',19:'s',20:'t',21:'u',22:'v',23:'w',24:'x',
25:'y',26:'z',27:'.',28:',',29:"'",30:'!',31:':',32:'1',
33:'2',34:'3',35:'4',36:'5',37:'6',38:'7',39:'8',40:'9',
41:'0',42:'=',43:'-',44:'"',45:'+',46:'[',47:'_',48:'(',
49:')',50:' ',51:'?',52:'\n',53:'@',54:'#',55:'/',56:'%',
57:'^',58:'*',59:']',60:'{',61:'}',62:'A',63:'B',64:'C',
65:'D',66:'E',67:'F',68:'G',69:'H',70:'I',71:'J',72:'K',
73:'L',74:'M',75:'N',76:'O',7

Solution

This is a good place for a list comprehension:

numKey = []
for keyClearChar in key:
        keyNum = dictDown[keyClearChar]
        numKey.append(keyNum)


Can be succinctly written as

numKey = [dictDown[char] for char in key]


Python has the nice feature that the for loop does not care what it iterates over, as long as it is iterable. This means, you don't have to convert your keys and texts to lists, for char in "AB CD" works perfectly fine.

Your encrypt and decrypt function differ only by the parameter names and the sign used in the actual shift. You could combine them into one function.

from itertools import cycle

def vigenere(key, text, decrypt=False):
    sign = -1 if decrypt else 1

    numKey = [dictDown[char] for char in key]
    out = (dictUp[(dictDown[char] + sign*keyIndex) % len(dictUp)]
           for keyIndex, char in zip(cycle(numKey), text))
    return ''.join(out)


I used itertools.cycle (EDIT: itertools.zip_longest fills the shorter with a given value, we want to repeat the key, therefore cycle), which makes manually keeping track of the keyCount obsolete. I also inlined the whole calculation, though it can of course be written in a more verbose for loop. It is first build as a generator object, which is passed to join. This avoids building the list (which has the same length as the text, which is potentially big), saving memory.

EDIT: The index calculation was wrong because of missing parenthesis before the modulus (Also in your code).

You can now, if you want to, define convenience functions, as well:

def crypt(key, text):
    return vigenere(key, text)

def decrypt(key, text):
    return vigenere(key, text, True)


You should try to avoid polluting the namespace too much. Prefer

import os
....
os.system('cls')


(This is a bit of a preference, I did not adhere to it myself in the above function. However, it makes it obvious where a function comes from, at the price of additional characters.)

You should make the path to the files a constant at the beginning of the code, helping implementing command line arguments for that at a later time.

Move all your code not in a function into a main function. Then at the end of the code use:

if __name__ == "__main__":
    main(TEXT_FILE)


where TEXT_FILE is the path to the file, obviously. This way you can import this script and use its functions at a later time (e.g. if you want to use the crypt function separately in another program).

If you want to make the path the first command-line argument you can now just do:

if __name__ == "__main__":
    text_file = sys.argv[1] if len(sys.argv) > 1 else TEXT_FILE
    main(text_file)


The built-in module string has some nice character classes. Your dictUp is basically just this:

import string
dictUp = dict(enumerate(string.printable[:95]))


where the :95 is there to exclude the last few (tab, newline, carriage return, etc). The order might be slightly different, so your code will not be downward compatible anymore if you make this change.

string.printable == '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!"#$%&\'()*+,-./:;?@[\\]^_`{|}~ \t\n\r\x0b\x0c'


dictDown is then just the inverted dictionary of that:

def invert(d):
    return {v: k for k, v in d.iteritems()}

dictDown = invert(dictUp)


Or use itertools.count and make dictUp a simple string (dictUp[3] is not different for a list, a string or a dict, with all keys in range(95) defined).

from itertools import count

# technically not a dict anymore, but sufficient for what you use it for:
dictUp = string.printable[:95]
dictDown = dict(zip(dictUp, count()))


Note here that most python implementations cache single-letter strings, so dictUp[35] should be O(1) both for a dict and a string.

Code Snippets

numKey = []
for keyClearChar in key:
        keyNum = dictDown[keyClearChar]
        numKey.append(keyNum)
numKey = [dictDown[char] for char in key]
from itertools import cycle

def vigenere(key, text, decrypt=False):
    sign = -1 if decrypt else 1

    numKey = [dictDown[char] for char in key]
    out = (dictUp[(dictDown[char] + sign*keyIndex) % len(dictUp)]
           for keyIndex, char in zip(cycle(numKey), text))
    return ''.join(out)
def crypt(key, text):
    return vigenere(key, text)

def decrypt(key, text):
    return vigenere(key, text, True)
import os
....
os.system('cls')

Context

StackExchange Code Review Q#139235, answer score: 5

Revisions (0)

No revisions yet.