XECryption encryption and decryption script

Question

The encryption script:

import random
splitArray = []
def takeInput():
 rawText = raw_input("Type something to be encrypted: \n")
 password = raw_input("Please type a numerical password: \n")
 encrypt(rawText, int(password))
def encrypt(rawText, password):
 for c in rawText:
 divide(c, password)
def divide(charToDivide, password):
 asciiValue = ord(charToDivide)
 a = 0
 b = 0
 c = 0
 passa = 0
 passb = 0
 passc = 0
 a = random.randrange(a, asciiValue)
 b = random.randrange(b, asciiValue - a)
 c = asciiValue - (a+b)
 passa = random.randrange(passa, password)
 passb = random.randrange(passb, password-passa)
 passc = password - (passa+passb)
 if isinstance(password, str): 
 print "Please enter a number"
 takeInput()
 else:
 a += passa 
 b += passb
 c += passc 
 splitArray.append(str(a))
 splitArray.append(str(b))
 splitArray.append(str(c))
takeInput()
f = open("myEncryptorTest", 'w')
arrayDelimiterString = "."
encryptedString = arrayDelimiterString.join(splitArray)
encryptedString = "." + encryptedString
f.write(encryptedString)
f.close

Decryption:

#XECryption decryption program
#Each character is a set of three numbers delimited by dots
#Decryption works by adding these three numbers together, subtracting the ASCII for a space and using that number to decypher 
#the rest of the array.
#User is prompted to see if the message makes sense
f = open('myEncryptorTest')
encryptedString = f.read()
f.close()
#separate sets of three numbers into an array
def sort():
 sortedCharArray = []
 charBuffer = "" 
 for c in encryptedString:
 if c == '.' and charBuffer != "":
 sortedCharArray.append(charBuffer)
 charBuffer = ""
 elif c != '.':
 charBuffer += c
 #if the buffer is not empty (e.g. last number), put it on the end
 if charBuffer != "":
 sortedCharArray.append(charBuffer)
 charBuffer = ""
 crack(sortedCharArray)
#add sets of three numbers together and insert into an array decryption
def crack(charArray):
 charBuffer = 0
 arrayCount = 1
 decypheredArray = []
 for c in charArray:
 if arrayCount % 3 == 0:
 arrayCount = arrayCount + 1
 charBuffer = charBuffer + int(c)
 decypheredArray.append(charBuffer)
 charBuffer = 0
 else:
 arrayCount = arrayCount + 1
 charBuffer = charBuffer + int(c)
 decypher(decypheredArray)
#subtract ASCII value of a space, use this subtracted value as a temporary password
def decypher(decypheredArray):
 space = 32
 subtractedValue = 0
 arrayBuffer = []
 try:
 for c in decypheredArray:
 subtractedValue = c - space
 for c in decypheredArray:
 asciicharacter = c - subtractedValue
 arrayBuffer.append(asciicharacter)
 answerFromCheck = check(arrayBuffer)
 if answerFromCheck == "y":
 #print value of password if user states correct decryption
 print "Password: "
 print subtractedValue 
 raise StopIteration()
 else:
 arrayBuffer = []
 except StopIteration:
 pass
#does the temporary password above produce an output that makes sense?
def check(arrayBuffer):
 decypheredText = ""
 stringArray = []
 try:
 for c in arrayBuffer:
 try:
 stringArray.append(chr(c))
 except ValueError:
 pass 
 print decypheredText.join(stringArray)
 inputAnswer = raw_input("Is this correct?")
 if inputAnswer == "y":
 return inputAnswer
 else:
 stringArray = []
 return inputAnswer
 except StopIteration:
 return 
sort()
f.close()

As I say, I'm looking for advice on how to improve my code and writing code in general. I'm aware that my code is probably an affront against programmers everywhere but I want to improve. These two scripts are for the hackthissite.org Realistic 6 mission. I won't be using them for encrypting anything of great importance.

• Jeff Welling: Unless you have a specific and very good reason for writing your own encryption algorithm, you most likely shouldn't do it. diovo.com/2009/02/wrote-your-own-encryption-algorithm-duh
• user331296: I wrote this for the hackthissite.org Realistic 6 mission.
• tshepang: codereview.stackexchange.com/…

Answer 1

import random
splitArray = []

It is best not to store data in global variables. Instead, you should have functions take all the input they need and return the result

def takeInput():
 rawText = raw_input("Type something to be encrypted: \n")

The python style guide recommends lowercase_with_underscores for local variable names

 password = raw_input("Please type a numerical password: \n")
 encrypt(rawText, int(password))

If the user enters something not a number, you'll get an exception. Its best to catch the exception and ask the user to try again

def encrypt(rawText, password):
 for c in rawText:
 divide(c, password)
 print c 

Why are you printing c? If its just debug output you should remove when you've finished debugging.

def divide(charToDivide, password):
 asciiValue = ord(charToDivide)
 a = 0
 b = 0
 c = 0

Consider storing these as a tuple

 passa = 0
 passb = 0
 passc = 0

And here as well

 a = random.randrange(a, asciiValue)

Ok, why did you assign zero to a earlier rather then just putting 0 as the argument here

 b = random.randrange(b, asciiValue - a)

Same here

 c = asciiValue - (a+b)
 passa = random.randrange(passa, password)
 passb = random.randrange(passb, password-passa)
 passc = password - (passa+passb)

And again here, the earlier assignments seemed to have no purpose. Additionally, the same logic was applied to both asciiValue and password. Make a function that produces the a,b,c from the input and call it for each of those values instead.

 if isinstance(password, str): 
 print "Please enter a number"
 takeInput()

Given your code how could password possibly be a string? If it were you would have already gotten an exception. You also shouldn't really mix function that perform logic and ones that take input. else: a += passa
b += passb c += passc
splitArray.append(str(a)) splitArray.append(str(b)) splitArray.append(str(c))

If you take my advice and put a,b,c in a tuple, then you could done this in one line.

takeInput()
f = open("myEncryptorTest", 'w')
arrayDelimiterString = "."
encryptedString = arrayDelimiterString.join(splitArray)

There isn't a whole lot of point in assigning a variable in one line and then using it only on the next.

encryptedString = "." + encryptedString

Instead of that:

encryptedString = ''.join('.' + data for data in splitArray)
f.write(encryptedString)
f.close

You are missing the (), so the file won't actually be closed.

#XECryption decryption program
#Each character is a set of three numbers delimited by dots
#Decryption works by adding these three numbers together, subtracting the ASCII for a space and using that number to decypher 
#the rest of the array.
#User is prompted to see if the message makes sense

Python has a feature called docstrings. If the first thing in a module or function is a string, it it considered the documentation for that object. You would do something like

"""
XEcryption decryption program
yada yada yada
"""

Rather then all those comments

f = open('myEncryptorTest')

I recommend against single-letter variable names

encryptedString = f.read()
f.close()
#separate sets of three numbers into an array

This comment should be a docstring in the function like I described. Also, the function doesn't do anything with sets of three numbers.

def sort():
 sortedCharArray = []
 charBuffer = "" 
 for c in encryptedString:
 if c == '.' and charBuffer != "":
 sortedCharArray.append(charBuffer)
 charBuffer = ""
 elif c != '.':
 charBuffer += c
 #if the buffer is not empty (e.g. last number), put it on the end
 if charBuffer != "":
 sortedCharArray.append(charBuffer)
 charBuffer = ""

This entire function can be replaced by sortedCharArray = encryptedString.split('.') Also, the function doesn't do any sorting.

 crack(sortedCharArray)
#add sets of three numbers together and insert into an array decryption
def crack(charArray):
 charBuffer = 0
 arrayCount = 1
 decypheredArray = []
 for c in charArray:
 if arrayCount % 3 == 0:

Don't iterate over what you have, iterate over what you want. Do something like:

 for index in xrange(0, len(charArray), 3):
 triplet = charArray[index:index+3]

That way triplet will have the three elements and you can operate on those without having to keep track of charBuffer or arrayCount

 arrayCount = arrayCount + 1
 charBuffer = charBuffer + int(c)
 decypheredArray.append(charBuffer)
 charBuffer = 0
 else:
 arrayCount = arrayCount + 1
 charBuffer = charBuffer + int(c)
 decypher(decypheredArray)

Rather then calling the next function in a chain like this. I suggest that you return at the end of each function and then have a master function pass that onto the next function. That way the master function will have a list of all of the steps in the algorithm. But bonus points for actually splitting it up into multiple functions

#subtract ASCII value of a space, use this subtracted value as a temporary password
def decypher(decypheredArray):
 space = 32

Why don't you use space = ord(' ')

 subtractedValue = 0
 arrayBuffer = []
 try:
 for c in decypheredArray:
 subtractedValue = c - space
 for c in decypheredArray:

Careful! your inner loop has the same variable as the outer loop asciicharacter = c - subtractedValue arrayBuffer.append(asciicharacter) answerFromCheck = check(arrayBuffer) if answerFromCheck == "y":

Use the python values True and False, not strings.

 #print value of password if user states correct decryption
 print "Password: "
 print subtractedValue 
 raise StopIteration()

Use break or return. You almost never need to raise StopIteration else: arrayBuffer = []

Rather then clearing the arrayBuffer here, create a new arrayBuffer except StopIteration: pass See this is problematic because something else could raise a StopIteration and you might accidently catch it.

#does the temporary password above produce an output that makes sense?
def check(arrayBuffer):
 decypheredText = ""
 stringArray = []
 try:
 for c in arrayBuffer:
 try:
 stringArray.append(chr(c))
 except ValueError:
 pass

If you get a ValueError, its because the character in question wasn't ascii. Shouldn't you just assume that the password was incorrect in that case?

Try stringArray = map(chr, arrayBuffer) instead of that loop.

 print decypheredText.join(stringArray)
 inputAnswer = raw_input("Is this correct?")

You are mixing input and logic. I recommend reworking your code so they are seperate.

 if inputAnswer == "y":
 return inputAnswer
 else:
 stringArray = []

This has no effect outside this function, why are you doing it?

 return inputAnswer
 except StopIteration:
 return 
sort()
f.close()

• user331296: This is exactly what I was after. Thanks for taking the time to reply.
• user331296: The password can be an ASCII character now, the encryption script basically adds the ASCII values of the characters together and uses that as a password. I'll go over the rest of the code with your advice.
• Xavier Combelle: @user331296 if you agree the answer please tag it as accepted in front of answer