Encrypter - Double Vigenere Cipher in Python

Question

I wrote this encrypter based on the idea of a Vigenere cipher, but instead of using only one key, it "makes" another key from the existing key. The length of the second key also depends on the different characters in the first key. And so it uses both keys in the shifting of letters.

def scram(key):
 key2 = []
 #make the length of the second key varying from key to key for harder cracking
 length = (key[0]-32)+(key[int(len(key)/2)]-32)+(key[len(key)-1]-32)
 #make the max length = 256
 length = length % 256
 #if the length is less than 64, multiply by two to make it longer
 while(length < 64):
 length*=2
 #scrambles the letters around
 for x in range(length):
 #basically shifts the key according to the current character,
 #how many times it has looped the key, where it is in the key,
 #and the modulo of x and a few prime numbers to make sure that 
 #an overlap/repeat doesn't happen.
 toapp = (x%(len(key)-1)) + (x/(len(key)-1)) + (key[x%(len(key)-1)]) + (x%3) +(x%5)+(x%53)+(x%7) 
 toapp = int(toapp % 94)
 key2.append(toapp)
 return key2
def cipher(mes,key,ac):
 #makes the second key
 key2 = scram(key)
 res=[]
 #do proper shifting in the keys
 for x in range(len(mes)):
 temp=mes[x]
 if(action == "2"):
 temp = temp - key[x%(len(key)-1)] - key2[x%(len(key2)-1)]
 else:
 temp = temp + key[x%(len(key)-1)] + key2[x%(len(key2)-1)] 
 temp = int(temp % 94)
 res.append(chr(temp+32))
 return res
#encrypt or decrypt
action = input("type 1 to encypt. type 2 to decrypt:")
#input
m= input("Text:")
k= input("key - 4 or more char:")
#changes the letters to ascii value
mes= []
for x in m:
 mes.append(ord(x)-32)
key= []
for x in k:
 key.append(ord(x)-32)
#encrypts it 
result = cipher(mes,key,action)
for x in result:
 print(x,end="")
print("")
y = input("Press enter to continue...")

Are there more efficient ways to do it? Is this a safe way to encrypt text? Can you crack the text encrypted with this program?

• jonrsharpe: These things are fun exercises, but if you actually want "a safe way to encrypt", don't roll your own!
• Caridorc: Are you sure you need y = input("Press enter to continue...")?
• Tanapoom Sermchaiwong: I put the last line there because it was very annoying when command prompt would just close immediately without letting me copy or see the output.
• Caridorc: Please do not change the code in the question. Post a follow-up implementing all the improvements

Answer 1

List comprehension

m= input("Text:")
k= input("key - 4 or more char:")
#changes the letters to ascii value
mes= []
for x in m:
 mes.append(ord(x)-32)
key= []
for x in k:
 key.append(ord(x)-32)

This all can be shortened to two list comprehensions:

mes = [ord(x) - 32 for x in input("Text: ")]
key = # Exercise for the reader

Avoid globals in functions

 if(action == "2"):

action should be a parameter of the function.

temp forever

 temp=mes[x]

You name a variable temp and use it 8 lines... please give a sensible name to this important variable.

Super long formula

toapp = (x%(len(key)-1)) + (x/(len(key)-1)) + (key[x%(len(key)-1)]) + (x%3) +(x%5)+(x%53)+(x%7) 

Please, put a comment near this huge formula.

Avoid so many assignments

result = cipher(mes,key)
for x in result:
 print(x,end="")
print("")

In python it is idiomatic to avoid extraneous variable assignements and explicit for loops are rarely really necessary, this all can be shortened to:

print(''.join(cipher(mes,key)))

Do not assign junk input

The last line should just be:

input("Enter to continue...")

• Caridorc: @holroy that would not help, it is needed to add a comment about why that formula was chosen
• Caridorc: @holroy I do not know if that would improve the code... efficiency is not a concern... you may suggest it in your answer and people may decide by voting

Answer 2

You don't need to iterate over a list like this:

for x in range(len(mes)):
 temp=mes[x]

If you need both the value and the index of the value then you can use enumerate. It will basically return tuples of both the value and it's index. It's usually used for for loops in cases like this. This is how you'd use it:

for x, temp in enumerate(mes):

This sets x the way you had before but also sets temp in the same line. It's more idiomatic to use i instead of x, but I left it as x to match your existing code. Also I do recommend using a better name for temp.

Also the Python style guid (PEP0008) recommends that you evenly space either side of an operator, so these lines:

m= input("Text:")
k= input("key - 4 or more char:")

Should have a space before the =

m = input("Text:")
k = input("key - 4 or more char:")

It also suggests that you put space between each value in a comma separated list.

result = cipher(mes, key, action)

This ending line, used to prevent the terminal closing without notice, should be clearer to the user and doesn't need to store the result.

input("Script finished")

Also I would recommend wrapping the code for reading input in a function. It's considered neater than loose code. What if you wanted to import this encrypter? It would just immediately run on import and start asking the user for input, which isn't necessarily what you want.