7
\$\begingroup\$

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?

\$\endgroup\$
  • 2
    \$\begingroup\$ These things are fun exercises, but if you actually want "a safe way to encrypt", don't roll your own! \$\endgroup\$ – jonrsharpe Aug 29 '15 at 18:48
  • \$\begingroup\$ Are you sure you need y = input("Press enter to continue...")? \$\endgroup\$ – Caridorc Aug 29 '15 at 22:16
  • \$\begingroup\$ 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. \$\endgroup\$ – Tanapoom Sermchaiwong Aug 30 '15 at 2:44
  • \$\begingroup\$ Please do not change the code in the question. Post a follow-up implementing all the improvements \$\endgroup\$ – Caridorc Aug 30 '15 at 8:15
7
\$\begingroup\$

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...")
\$\endgroup\$
  • \$\begingroup\$ @holroy that would not help, it is needed to add a comment about why that formula was chosen \$\endgroup\$ – Caridorc Aug 29 '15 at 22:15
  • \$\begingroup\$ @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 \$\endgroup\$ – Caridorc Aug 29 '15 at 22:21
3
\$\begingroup\$

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.

\$\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.