Encrypter and decrypter project

Question

I am here to ask for criticism, as for my first week of Python I feel like I have improved a lot. I recreated the first program I made within my first hour with a tutorial, the one I made now is based on the knowledge I have gained within the week. It is my first language so I am sorry if the syntax is confusing

I really really want to improve and have the ability feel like home in Python. Having my imagination as limits instead of not knowing how to implement it in code.

First Project

The code is a mess here with it having

• no functions
• messy variables
• non reusable code
• running on the same file.

import random
import string

chars = " " + string.punctuation + string.digits + string.ascii_letters
chars = list(chars)
keys = chars.copy()

'''
Fully aware that I can just use random.shuffle(keys)
but I want to be able to decrypt the messages without 
it randomizing each time I use it, can save in text file
but it will be inaccessible to others that don't have the
txt file.
'''

# Sets of keys that the randInt will choose from
keys1 = [';', 'S', 'p', 'Q', 'O', 'f', 'X', 'x', ')', 'Y', 'Z', 'I', 'q', '2', '(', 'e', '3', 'w', 'R', '5', 'u', 'n', '7', '#', '4', '/', 'l', '_', 'd', "'", '?', '|', ',', 'T', 'E', 'F', 'k', '\\', 'A', '}', 'j', 'r', 't', '0', '&', 'y', '%', ']', '<', 'v', 'C', 'N', '9', 'W', 'D', '$', '^', '*', 'i', '{', '-', ' ', '6', 'H', 'P', 'G', '~', 'a', ':', 'M', 's', 'J', 'z', 'm', 'U', '"', 'K', '[', 'L', 'o', '+', 'c', '@', '!', '=', '8', '.', 'V', 'h', 'b', '`', '1', 'g', 'B', '>']
keys2 = ['d', ']', '(', '.', "'", 'P', 'l', 'j', 'g', 'u', 'E', '{', '-', 'q', '~', 'o', '[', '9', 'V', ',', 'y', '\\', '1', '/', 'a', 'F', 'n', 'M', 'p', '6', 'z', 'H', 'v', '7', 'Z', 'Y', '5', 'W', '>', '`', '|', '$', '#', ':', 'i', 'X', '*', '%', 'D', '"', 'c', ' ', 'C', '^', 'k', '4', 'B', 'b', '}', 'A', 'Q', 's', 'I', ';', 'm', 'N', 'K', 'f', 'r', 'G', '0', '3', '!', 'J', '<', '8', ')', 'R', 'L', '=', '_', 'S', '&', 't', 'U', 'h', '+', 'w', '2', 'x', 'O', 'T', 'e', '?', '@']

# Random integer
randInt = ""
randInt = random.randint(1,2)

# Determines key use
keyUse = ""

# Sets the keys used ( Adds the determiner too "!" or "?")
if (randInt == 1): 
    encr_T = "!"
    keyUse = keys1
elif(randInt == 2):
    encr_T = "?"
    keyUse = keys2

# Encrypt
char_T = input(f"Encrypt : ")

# Indexing
for letter in char_T:
    index = chars.index(letter)
    encr_T += keyUse[index]

print(f"Encrypted text : {encr_T}")

# Decrypt
encr_T = input("Decrypt : ")
char_T = ""

# Indexing
for letter in encr_T:
    if (encr_T[0] == "!"):
        index = keys1.index(letter)
    elif (encr_T[0] == "?"):
        index = keys2.index(letter)
    
    char_T += chars[index]
        
print(f"Decrypted text : {char_T[1:]}")

Current progress remake

Having watched a couple of videos, read a couple of source codes, and learned a couple of other alternative ways to approach code, I applied

• avoiding nesting
• having actual functions to make code reusable
• keeping the long variable values in another file to keep the code clean

(I am aware of the random.shuffle() function but I wanted to give it the ability to decrypt what I encrypted so I set an indicator of which set of keys are being used, which is why I have the 'encryption key' variable in key_checker() and the decrypted text being printed from the index [:1].)

• use of dictionaries to avoid a longer code
• annotations to help me understand the parameters I am giving in

I also have main.py that is running the code.

main.py

from work import user_input as up
if __name__ == '__main__':
    while True:
        up()

keys.py

import random 
import string

key_choice = random.randint(1,3)
key1 = " " + string.punctuation + string.digits + string.ascii_letters
key_1 = list(key1)
key_2 = ['J', '0', '>', '(', '$', 'w', '/', '_', 'N', ',', 'c', ':', '+', '{', '2', '"', '|', '<', '5', 'v', '9', '\\', '&', ')', '~', 'Y', 'W', 'C', 'u', 'b', 'h', 'R', ';', 'o', 'U', '4', 'B', 'Z', '-', '6', 's', '}', 'E', 'y', '*', '@', 'V', '1', 't', 'n', 'm', '^', '[', '%', 'F', 'K', 'T', '?', ']', ' ', 'q', '`', 'X', '=', 'd', 'g', 'G', '7', '!', 'D', 'r', '3', 'x', 'A', 'l', 'H', '.', 'S', '8', 'j', 'L', 'M', 'O', 'a', 'Q', 'P', "'", 'z', 'i', 'e', 'k', '#', 'p', 'I', 'f']
key_3 = ['6', 'H', 'j', 'B', 'L', '~', 'F', '"', 'c', '>', '5', '3', '<', '9', 'w', '=', 'X', 'r', ')', '&', 'S', 'm', '/', 'x', 'u', 'W', 'n', '\\', 's', 'D', 'l', 'f', 'J', 'q', '0', 'z', 'I', 'M', 'A', '{', 'E', 'P', '|', 'Y', 'y', '^', '%', '#', 'k', '+', '?', 'Q', 'e', "'", ']', '.', 'G', '(', 'b', 't', '1', '!', 'R', 'V', '*', '4', 'O', ';', '-', '$', 'Z', 'i', 'a', '_', '[', ':', 'N', ',', '}', 'C', 'T', 'g', 'v', 'o', 'h', ' ', 'd', 'U', 'K', '8', '7', 'p', '@', '`', '2']
key_4 = ['B', 'o', 'C', '#', 'j', ':', 'S', '}', '<', 'W', 'l', '2', ')', 'f', '"', 'z', 'M', '`', '{', 'U', 'a', 'y', 't', 'p', 'm', '8', 'X', 'I', '6', 'w', 'Z', '.', 'G', '@', '3', 'N', '(', '!', ']', '_', '+', 'O', '^', ',', '0', 'u', '5', 'e', 'q', 'n', 'x', 'i', 'b', 'Y', '7', 'g', 'v', '[', ';', '>', '$', 'Q', '/', 'E', ' ', 'R', 'V', 'r', 'T', 'J', '=', '1', 'D', 's', '?', '~', 'h', '-', '&', '|', 'c', 'A', '*', 'k', '9', '\\', 'K', 'P', 'F', '4', 'H', '%', "'", 'd', 'L']

key_dict = {
    '!': key_2,
    '?': key_3,
    '@': key_4,
}

work.py

from keys import key_1 as k1, key_2 as k2, key_3 as k3, key_4 as k4, key_choice as ks, key_dict as kd

def key_checker() -> tuple[str, list[str]]:
    if ks == 1:
        encryption_key = "!"
        key_use = k2
    elif ks == 2:
        encryption_key = "?"
        key_use = k3
    else:
        encryption_key = "@"
        key_use = k4
    return encryption_key, key_use

def user_input() -> str:
    encryption_key, key_use = key_checker()
    message_to_encrypt = input('\nEncrypt / Decrypt / Quit? : ').lower()
    if message_to_encrypt == 'quit':
        exit()
    elif message_to_encrypt == 'encrypt':
        encr_this = input('Encrypt this message : ')
        start_encrypt(encr_this, encryption_key, key_use)
    elif message_to_encrypt == 'decrypt':
        decr_this = input('Decrypt this message : ')
        start_decrypt(decr_this)
    else:
        print('Invalid Response!')
        
def start_encrypt(message_to_encrypt: str, encryption_key: str, key_use: list[str]) -> str:
    for i in message_to_encrypt:
        index = k1.index(i)
        encryption_key += key_use[index]
    print(f"\nEncrypted message : {encryption_key}")
        
def start_decrypt(message_to_decrypt: str) -> str:
    clear_text = ""
    key_use = kd.get(message_to_decrypt[0])
    for i in message_to_decrypt:
        index = key_use.index(i)
        clear_text += k1[index]
    print(f"Decrypted message : {clear_text[1:]}")

I have tried optimizing my code into something that runs better and more understandable for others, trying to improve in Python too.

Answer 1

type stability

Welcome!

This is maybe a subtle point but I will raise it here.

chars = " " + string.punctuation + string.digits + string.ascii_letters
chars = list(chars)

We started out with chars: str, and then changed our mind and made the type chars: list[str]. So, chars doesn't have a stable type; the meaning of that identifier changes with each line.

Prefer chars = list(" " + string. ... + ... + ... ). Then it only has a single meaning from the get go.

BTW, fun fact, here's another way to obtain the same characters:

chars = sorted(set(string.printable) - set(string.whitespace) | set(" "))

Similarly, we redefine the str randInt to later be an int. Calling it a string just seems bizarre.

lint

Pep-8 asks that you name it rand_int.

Similarly for key_use, encr_t, char_t, etc.

I recommend you run $ black -S *.py on your source every now and again, to tidy up the inconsistent use of whitespace.

discarded str constant is not a comment

Please phrase it this way:

# Fully aware that I can just use random.shuffle(keys)
# but I want to be able to decrypt the messages without 
# it randomizing each time I use it, can save in text file
# but it will be inaccessible to others that don't have the
# txt file.

It is syntactically valid to compute 5, or 2 + 3, or 'foo' on a line by itself, and then discard the result. But in python the """docstring""" goes in a special place: right after a {function, method, class, module} definition, typically the next line after the parameters in a signature. Put # comments where they belong -- in the middle of code. Avoid computing a constant expression only to immediately discard it.

a list is not a set

# Sets of keys that the randInt will choose from

A set in python is a very specific datastructure, but we have a pair of lists, here. Better to phrase this as # Keys that ....

Also, it would be more convenient to phrase those as

keys1 = list(';SpQ...')
keys2 = list( ... )

comments describe the why, not the how

# Determines key use
keyUse = ""

The comment doesn't illuminate the code. We don't know anything after reading the comment beyond what the code already told us. Recommend you just elide the comment.

Let's revisit the comment above.

# Sets of keys that the randInt will choose from

Now, that is accurate. But here is what's really interesting about the relationship between those two constants:

>>> set(keys1) == set(keys2)
True

It would be worth pointing out that they are permutations of each other. That is, a comment should say things that aren't completely obvious from just a glance at the code.

extra parens

if (randInt == 1): 
    ...
elif(randInt == 2):

This isn't C or java -- please elide those superfluous ( ) parens.

BTW, a simple else: would have sufficed, no biggie.

terms from the Business Domain

print(f"Encrypted text : {encr_T}")

Consider renaming that identifier. Cryptographers have a standard term for it: ciphertext. Similarly char_t is plaintext.

In general, when reading a Specifications or Requirements document or some cited reference, when you see it mention a defined term, that tends to be a good name for when you need a new python identifier.

define a function

print(f"Decrypted text : {char_T[1:]}")

Awesome! We round-tripped from plaintext to ciphertext and back to plaintext again, mission accomplished.

Now would be a good time to package up that code within a pair of functions.

def encrypt(plaintext: str) -> str:
    ...
def decrypt(ciphertext: str) -> str:
    ...

Passing strings back and forth is more convenient than evaluating code for print() side effects:

plaintext = input(f"Encrypt : ")
print("ciphertext: ", encrypt(plaintext))
print("plaintext:  ", decrypt(encrypt(plaintext)))

meaningful identifier

from work import user_input as up

That rename is a bit cryptic. The original name was lovely and descriptive. Recommend you retain it:

from work import user_input

generalize

key_2 = ['J', ... ]
key_3 = ['6', ... ]
key_4 = ['B', ... ]

This is starting to become a tediously long copy-n-paste exercise. When you notice something like that happening, you might take a step back and ask "how can we generalize that operation?" -- the randomish .shuffle() in this case.

Write a function which accepts a seed integer, puts the PRNG in a known state, and returns a shuffled copy of its input. That way, if someone runs it today or tomorrow, they will always get back the same list of characters. (And you may as well write it out to a .txt file, as well, if only for debugging purposes.)

renaming identifiers

from keys import key_1 as k1, key_2 as k2, key_3 as k3, key_4 as k4, key_choice as ks, key_dict as kd

Again, the original identifiers were lovely, and giving them cryptic aliases doesn't seem helpful here.

Also, based on the logic in key_checker(), you are finding it inconvenient to have four separate variables. A single dict with four keys would be a better fit.

docstrings

You chose great identifiers for key_checker(), user_input(), and start_encrypt(). But they are not completely self-explanatory. It would be a really good idea to put a one-sentence """docstring""" after each of those three function signatures. We want to know what the single responsibility of each function is. What promise does it make, what will be true upon return?

type annotations

You are apparently not using the mypy type checker.

def user_input() -> str:
    ...
        start_decrypt(decr_this)
    else:
        print('Invalid Response!')

You promised to return a str, but instead we fell off the end so we implicitly return None. The mypy linter would have caught this "whoops!" for you.

Similarly for start_encrypt() and start_decrypt(), which promise to return a str but instead produce a print() side effect. (Not sure what the start_ prefix is all about, since the complete operation is performed.)

As long as you're linting, you may as well run $ ruff *.py, as well. It's similar to pylint, but faster, and it's not too picky so it won't get in your way. If it calls out some detail, it's usually worth your while to attend to it so the code will lint clean.

Also, avoid exit() where a simple return would suffice. It's a bit more violent than necessary, tearing down the whole cPython interpreter so caller never gets a chance to respond to what happened.

I do really like how input( ... ).lower() will downcase the user's input from the get go. Nice.

---

This code substantially achieves its design goals.

It is written in a clear style with helpful identifier spellings and comments. I would be willing to delegate or accept maintenance tasks on it.

Answer 2

This review concerns the first code block under the "First Project" heading.

In general, the code layout is good and you chose meaningful names for the variables. The flow of the code is straightforward and easy to understand.

It would be good to have comments at the top of the file to describe the purpose of the code and how to use it. There is no need to keep comment block that you have near the top since it looks like it describes changes you plan to make to the code, rather than describing the code you have.

Some of the comments in your code are unnecessary since they are redundant with the code that follows. For example:

# Random integer
randInt = ""

It is obvious from the variable name that you are initializing a random variable.

The line where you initialize the keys1 variable is much too long and can be split into several lines. The same is true for keys2.

Running pylint on your code is a good way to automatically point out style issues with the code.

For example, there is no need for the parentheses around the if conditions. Also, it is a good idea to remove trailing whitespace from the line of code.

Another pylint tip for the Encrypt input line is:

Using an f-string that does not have any interpolated variables

Here is a re-write with many of the above suggestions:

'''
Input a text string to encrypt and decrypt.
'''

import random
import string

chars = " " + string.punctuation + string.digits + string.ascii_letters
chars = list(chars)
keys = chars.copy()

# Sets of keys that the randInt will choose from
keys1 = [';', 'S', 'p', 'Q', 'O', 'f', 'X', 'x', ')',
        'Y', 'Z', 'I', 'q', '2', '(', 'e', '3', 'w',
        'R', '5', 'u', 'n', '7', '#', '4', '/', 'l',
        '_', 'd', "'", '?', '|', ',', 'T', 'E', 'F',
        'k', '\\', 'A', '}', 'j', 'r', 't', '0', '&',
        'y', '%', ']', '<', 'v', 'C', 'N', '9', 'W',
        'D', '$', '^', '*', 'i', '{', '-', ' ', '6',
        'H', 'P', 'G', '~', 'a', ':', 'M', 's', 'J', 
        'z', 'm', 'U', '"', 'K', '[', 'L', 'o', '+', 
        'c', '@', '!', '=', '8', '.', 'V', 'h', 'b', 
        '`', '1', 'g', 'B', '>']
keys2 = ['d', ']', '(', '.', "'", 'P',
        'l', 'j', 'g', 'u', 'E', '{', '-', 'q',
        '~', 'o', '[', '9', 'V', ',', 'y',
        '\\', '1', '/', 'a', 'F', 'n', 'M',
        'p', '6', 'z', 'H', 'v', '7', 'Z', 'Y',
        '5', 'W', '>', '`', '|', '$', '#', ':',
        'i', 'X', '*', '%', 'D', '"', 'c', ' ',
        'C', '^', 'k', '4', 'B', 'b', '}', 'A',
        'Q', 's', 'I', ';', 'm', 'N', 'K', 'f',
        'r', 'G', '0', '3', '!', 'J', '<', '8',
        ')', 'R', 'L', '=', '_', 'S', '&', 't',
        'U', 'h', '+', 'w', '2', 'x', 'O', 'T',
        'e', '?', '@']

randInt = ""
randInt = random.randint(1,2)

keyUse = ""

# Sets the keys used (Adds the determiner too "!" or "?")
if randInt == 1:
    encr_T = "!"
    keyUse = keys1
elif randInt == 2:
    encr_T = "?"
    keyUse = keys2

char_T = input("Encrypt : ")

# Indexing
for letter in char_T:
    index = chars.index(letter)
    encr_T += keyUse[index]

print(f"Encrypted text : {encr_T}")

encr_T = input("Decrypt : ")
char_T = ""