Implementing an encryption standard over message text

Question

I wrote the below code and tested some example files on it. I didn't find any errors, but I still want to be thorough about this.

Also, any suggestions in improving the code?

import string
""" this implements an encryption standard over the message text
it replaces the odd place alphabets by their ordinal number plus place number increased.
it replaces the even place alphabets by their ordinal number minus place number decreased.
for example "hello" becomes "icoht" and "abc" becomes "bzf".
"""
input_filename = raw_input("Enter the input file name: ")
output_filename = raw_input("Enter the output dumping file name: ")
mode = raw_input("Do you want to encrypt or decrypt (e/d)?: ")
with open(input_filename,'r') as inputfile:
    with open(output_filename,'w') as outputfile:
        inp = inputfile.read()
        encode = lambda s: "".join(
            (chr(ord('a')+((ord(c)-(ord('a')+i+1))%26))) if ((c.isalpha()) and ((i+1)% 2 ==0))
            else
            string.ascii_lowercase[(string.ascii_lowercase.rfind(c.lower())+i+1)%26]  if ((c.isalpha()) and ((i+1)% 2 ==1))
            else
            c
            for i,c in enumerate(s.lower())
            )
        decode = lambda s: "".join(
            string.ascii_lowercase[(string.ascii_lowercase.rfind(c.lower())+i+1)%26] if ((c.isalpha()) and ((i+1)% 2 ==0)) 
            else
            (chr(ord('a')+((ord(c)-(ord('a')+i+1))%26))) if ((c.isalpha()) and ((i+1)% 2 ==1))
            else
            c
            for i,c in enumerate(s.lower())

            )
        if(mode.lower() == 'e'):
            outputfile.write(encode(inp))
        else :#(mode.lower() =='d'):
            outputfile.write(decode(inp))
    s = {'e':'Encoding','d':'Decoding'}
    print s.get(mode)+" Sucessfull"

Answer 1

Functions are your friend. The remove repeated code and improve readability. I have absolutely no idea what the block of text does inside the "".join(XXXX) calls. There is too much going on there that I'm not going to take the time to see what it does or if it can be done better.

However, it does appear that you can replace that mess with something like this:

def doA():
  pass
def doB():
  pass
def makeJoinArgument(value, which_step,step1, step2):
  return step1() if (which_step) else step2() for x in value
def encode(value):
  return "".join("abc", True, doA, doB)
def decode(value):
  return "".join("abc", True, doB, doA)

This obviously doesn't run (and not exactly what you have), but it is much more manageable than what you have now.

Another note (from PEP 8: Style Guide)

Always use a def statement instead of an assignment statement that binds a lambda expression directly to a name.

Yes:

def f(x): return 2*x

No:

f = lambda x: 2*x

The first form means that the name of the resulting function object is specifically 'f' instead of the generic ''. This is more useful for tracebacks and string representations in general. The use of the assignment statement eliminates the sole benefit a lambda expression can offer over an explicit def statement (i.e. that it can be embedded inside a larger expression)

Answer 2

Your encode and decode functions are absurdly long for 'one liners'. Also they contain these two lines

(chr(ord('a')+((ord(c)-(ord('a')+i+1))%26))) if ((c.isalpha()) and ((i+1)% 2 ==0))

and

string.ascii_lowercase[(string.ascii_lowercase.rfind(c.lower())+i+1)%26]  if ((c.isalpha()) and ((i+1)% 2 ==1))

which both do similar things in a different and needlessly complicated way.

A more readable and simpler version of the encode function would be:

def encode(s):
    r = list(s.lower())
    abc = string.lowercase()
    for i, c in enumerate(r):
        if c in abc:
            if i % 2:
                r[i] = abc[(abc.index(c) - i - 1) % 26]
            else:
                r[i] = abc[(abc.index(c) + i + 1) % 26]
    return ''.join(r)

In terms of structure you probably don't want to have the encode/decode functions defined inside the with in your main function, but separately:

import string
def encode(s):
    ...

def decode(s):
    ...

def main():
    """ this implements etc.
    """
    input_filename = raw_input("Enter the input file name: ")
    output_filename = raw_input("Enter the output dumping file name: ")
    mode = raw_input("Do you want to encrypt or decrypt (e/d)?: ").lower()
    with open(input_filename,'r') as inputfile:
        with open(output_filename,'w') as outputfile:
            inp = inputfile.read()
            if mode is 'e':
                outputfile.write(encode(inp))
                print 'Encoding successful'
            else :#(mode.lower() =='d'):
                outputfile.write(decode(inp))
                print 'Decoding successful'