7
\$\begingroup\$

I'm new to Python, but I really want to get into the swing of doing things the pythonic way. First, does my code follow the pythonic path? Or does it run screaming bloody murder, as it zig-zags through my C++ thought of mind? Probably the latter.

 import sys, random, string
 #import pdb
 #pdb.set_trace()

 def read(name):
     inFile=open(name, 'r')
     content=inFile.read()
     content=list(content.strip())
     return content

 #this function will take a file name and a list of numbers
 #it will convert the nums to chars and write to file 
 def write(name, nList):          
     outFile=open(name, 'w')
     cList=convertToChar(nList)
     outFile.write(''.join(cList))


 #takes a key length, based on message size and gens rand key
 def gen(keylen):
     i=0
     key=[]
     while i <keylen:
         k=random.SystemRandom()
         k=k.choice(string.printable)
         key.append(k)       #append random letter to key list
         i+=1
     key=convertToNum(key)    #conver the key to numbers for xor later
     write("key.txt", key)
     return key              #for xor'ing later in enc



 #takes a list of chars, converts to nums, and writes to list
 #returns a numList
 def convertToNum(charList):
     numList=[]
     i=0
     for i in charList:
         numList.append(ord(i))
     return numList

 #takes list of nums, converts to chars, and writes to new list
 #retursn the charlist
 def convertToChar(numList):
     charList=[]
     i=0
     for i in numList:
         charList.append(chr(i))
     return charList

 #This will ask the user for a file name
 #converts the 
 def enc():
     fileName= raw_input("Please enter a name for your message file")
     #fileName="plaintext.txt"
     plaintext=read(fileName)      #gets a list form of the file
     m= convertToNum(plaintext)    #the message in nums
     mLen=len(m)                   #len of message for size of key
     k=gen(mLen)                   #generate the key                         
     c=[]                          #list to srote the encrypted message
     c= [a^b for a, b in zip(m,k)] #xor's each element in the tuple (m,k)
     write("ciphertext.txt", c)   #writes the xor'ed message to teh cipher 

 def dec():
     ciphertext=read("ciphertext.txt")
     key=read("key.txt")
     ciphertext=convertToNum(ciphertext)
     key= convertToNum(key)
     i=0
     decrypted=([a^b for a, b in zip(ciphertext, key)])   
     write("decrypted.txt", decrypted)

 def main():
     enc()
     dec()

  main()
\$\endgroup\$
11
\$\begingroup\$

Here are some of your functions translated to be more pythonic:

Context managers mean never having to say your closed():

We can recast thse 2 functions (including fixing the bug of not closing the files):

def read(name):
    inFile = open(name, 'r')
    content = inFile.read()
    content = list(content.strip())
    return content

def write(name, nList):
    outFile = open(name, 'w')
    cList = convertToChar(nList)
    outFile.write(''.join(cList))

to:

def read(name):
    with open(name, 'r') as f:
        return f.read().strip()

def write(name, n_list):
    with open(name, 'w') as f:
        f.write(''.join(convertToChar(n_list)))

big key here is using context managers to automatically close the file, and not bothering to assign intermediate results to a variable.

List comprehensions can save a lot of looping boilerplate:

We will trim down:

def gen(keylen):
    i = 0
    key = []
    while i < keylen:
        k = random.SystemRandom()
        k = k.choice(string.printable)
        key.append(k)
        i += 1
    key = convertToNum(key)
    write("key.txt", key)
    return key

def convertToNum(charList):
    numList = []
    i = 0
    for i in charList:
        numList.append(ord(i))
    return numList

def convertToChar(numList):
    charList = []
    i = 0
    for i in numList:
        charList.append(chr(i))
    return charList

To:

def gen(keylen):
    key = convertToNum([random.SystemRandom().choice(string.printable)
                        for k in range(keylen)])
    write("key.txt", key)
    return key

def convertToNum(charList):
    return [ord(i) for i in charList]

def convertToChar(numList):
    return [chr(i) for i in numList]

All three of these functions now make use of list comprehensions to remove 75% of the lines of code.

NOTE

I did not test these changes they were just typed out in the editor.

\$\endgroup\$
  • 1
    \$\begingroup\$ You could also drop the list call in read as strings are iterables. \$\endgroup\$ – Mathias Ettinger Mar 18 '17 at 8:22
5
\$\begingroup\$

Stephen did a good job in his answer to make your functions more Pythonic. The only thing left to do now, is make them conform to the coding standards of Python. Python has an official style-guide, PEP8, which programmers are recommended to adhere to.

Specifically, your code does not follow it in these points:

  1. Functions should be named using lower_case (and variables as well)
  2. Operators should be surrounded by spaces in almost all cases (so 1 + 1 = 2, x = 3, but f(a, b, b) and f(a, b=3, c=4).
  3. A consistent number of spaces per tab should be used (PEP8 recommends using 4). I think you not following it here is mostly a problem of copy&paste.
  4. Functions (and classes) should be followed by two blank lines (you mix using one and two).

In addition to that, Python also has a convention for docstrings, which explain what a function does. The convention is set in PEP257 and looks like this:

def f(a, b):
    """A short description of the function"""
    return a + b

Or, if you need more space, something like this:

def g(a, b, c):
    """"
    A complicated function (not!)

    Args:
        a (float):  parameter a
        b (float):  parameter b
        c (float):  parameter c

    Returns:
        The sum of the parameters (float)
    """
    return a + b + c

Where the exact form of documenting the arguments and parameters is not set in stone. The above docstring roughly follows Google's docstring style-guide.

These docstrings are for example accesible by calling help(f) in an interactive Python session and third-party documentation tools will also automatically pick them up. They are stored in the attribute of the object, so you can also inspect them via f.__doc__.

Note that the the indentation of the first line is subtracted, so that you can nicely align the docstring below the function def and don't have to write:

def g(a, b, c):
    """"
A complicated function (not!)

Args:
    a (float):  parameter a
    b (float):  parameter b
    c (float):  parameter c
Returns:
    The sum of the parameters (float)
    """
    return a + b + c

And the last style remark, in Python we have a if __name__ == "__main__" guard, that ensures your code is only run when you call it via python script.py but not if you do from script import * in another script.

One final code remark. In your function dec (which could be called decode for clarity), you don't need the parenthesis around the list. Or, you don't need the list and can just make it a generator expression:

def decode():
    ciphertext = convert_to_num(read("ciphertext.txt"))
    key = convert_to_num(read("key.txt"))
    return (a^b for a, b in zip(ciphertext, key))   

And one structure comment. It is usually preferred to have (mostly) pure functions that take an input an return an output. So I would do:

def encode(text, key):
    return (a^b for a, b in zip(text, key))


def main():
    file_name = raw_input("Please enter a name for your message file")
    plaintext = convert_to_num(read(file_name))

    key = gen(len(plaintext))         
    write("ciphertext.txt", encode(plaintext, key))

    ciphertext = convert_to_num(read("ciphertext.txt"))
    key = convert_to_num(read("key.txt"))
    write("decrypted.txt", encode(ciphertext, key))


if __name__ == "__main__":
    main()

Here I used the fact that encode and decode are exactly the same, as soon as you factor out input and output. Also note that the blocks in main could now be factored out again to functions, but I opted for not doing this here for brevity.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.