3
\$\begingroup\$

At the moment I'm learning Python and I made this script to workout my skills. Since it is one of my first programs I would love some feedback and tips, like more organization, less lines of code or more optimization, anything I could improve.

print "Do you want to encrypt or decrypt the message (e/d)"
action = raw_input("> ")
while action != "e" and action != "d":
    action = raw_input( "(e/d): ")

abc_l_e = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
abc_u_e = "ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ"
abc_l_d = "zyxwvutsrqponmlkjihgfedcbazyxwvutsrqponmlkjihgfedcba"
abc_u_d = "ZYXWVUTSRQPONMLKJIHGFEDCBAZYXWVUTSRQPONMLKJIHGFEDCBA"
chars = "!\"#$%&'()*+-'-./:;<=>?@[]^_{}|~1234567890"

if action == "e":
    print "\nEnter the message to encrypt"
    message = raw_input("> ")

    print "\nEnter the key"
    try:
        key = input("> ")
        while not (key >= 1 and key <= 26):
            print "Number not in range (1-26). Try again"
            key = input("> ")
    except:
        print "That's not a correct integer. Try again."
        exit()

    e_message = ""
    for char in message:
        if char in abc_l_e:
            e_message += abc_l_e[(abc_l_e.index(char) + key)]
        if char in abc_u_e:
            e_message += abc_u_e[(abc_u_e.index(char) + key)]
        elif char == " ":
            e_message += " "
        elif char in chars:
            e_message += char
    print "Your encrypted message is:\n", e_message

elif action == "d":
    print "\nEnter the message to decrypt"
    message = raw_input("> ")
    print "\nEnter the key (enter \"n\" if you don't know)"
    try:
        key = raw_input("> ")
        if key != "n":
            while not (int(key) >= 1 and int(key) <= 26):
                print "Number not in range (1-26). Try again"
                key = input("> ") 
    except:
        print "That's not a correct integer nor \"n\". Try again."
        exit()

    d_message = ""
    if key != "n":
        key = int(key)  
        for char in message:
            if char in abc_l_d:         
                d_message += abc_l_d[(abc_l_d.index(char) + key)]
            elif char in abc_u_d:
                d_message += abc_u_d[(abc_u_d.index(char) + key)]
            elif char == " ":
                d_message += char
            elif char in chars:
                d_message += char
        print "Your decrypted message is:\n", d_message

    if key == "n":
        results = []
        for turn in range(1,26):
            d_message = ""
            for char in message:
                if char in abc_l_d:
                    d_message += abc_l_d[(abc_l_d.index(char) + turn)]
                elif char in abc_u_d:   
                    d_message += abc_u_d[(abc_u_d.index(char) + turn)]
                elif char == " ":
                    d_message += char
                elif char in chars:
                    d_message += char
            results.append(d_message)

        print "\nYour decrypted message is between one of the following:"
        for result in results:
            print "%d." % (results.index(result) + 1), result
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$
  • Rather than prompting for input, almost all shell scripts (including this) would be improved by accepting the static input (action and key) as arguments and then reading standard input to EOF to produce the output. For this you'll want a main method which just orchestrates argument parsing and passing the resulting values to the function or method which does the actual substitution. This makes it very easy to combine the script with other commands in a common way. argparse is very well suited for this.
  • Encrypting vs decrypting is simply a matter of shifting the characters left vs right. You can use this information to halve the code.
  • The brute force method should be implemented by running a single function 26 times.
  • In general, the code should be split into functions until there is no obvious duplication anywhere. This should massively simplify everything except the input handling, meaning that the interesting part of the script should end up being maybe three lines.
  • You should handle input one line at a time, so that even huge input results in small memory use.
\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the suggestions! I just don't get two things: I can't undeerstand your alternative to prompting for input, can you please give me an example? Also, what do you mean by handle input one line at a time? \$\endgroup\$
    – user143246
    Commented Jul 13, 2017 at 15:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.