I would greatly appreciate your feedback on my first ever Python project! :D
Basically I am coding a Caesar Cipher and I think its pretty terribly 'optimised / efficient' if you know what I mean, this is because I copied and pasted the encrypt() method for the decrypt() method and the only thing I changes was instead of rotating the numbers more, I rotated them less. This is what I'm talking about:
newPosition = (abc.find(letter) - key) % 26
^^ Instead of having a + (plus) I made it a - (minus) ^^
Is there a way I can sort of call the encrypt() method at just the newPosition line? Or what I did was correct and it doesn't need fixing (which I highly doubt)
** Please do take in mind that I do not have much knowledge in Python (if any at all) since I just started today so don't blow my brain up with some super complex code. THANK YOU!!! **
abc = 'abcdefghijklmnopqrstuvwxyz'
def main():
message = input("Would you like to encrypt or decrypt a word?")
if message.lower() == "encrypt":
encrypt()
elif message.lower() == "decrypt":
decrypt()
else:
print("You must enter either 'encrypt' or 'decrypt'.")
main()
def encrypt():
message = input("Enter a message to encrypt: ")
message = message.lower()
key = int(input("What number would you like for your key value?"))
cipherText = ""
for letter in message:
if letter in abc:
newPosition = (abc.find(letter) + key) % 26
cipherText += abc[newPosition]
else:
cipherText += letter
print(cipherText)
return cipherText
def decrypt():
message = input("Enter a message to decrypt: ")
message = message.lower()
key = int(input("What number would you like for your key value?"))
cipherText = ""
for letter in message:
if letter in abc:
newPosition = (abc.find(letter) - key) % 26
cipherText += abc[newPosition]
else:
cipherText += letter
print(cipherText)
return cipherText
main()
In general,
str.find
is bad performance-wise. It's O(n) complexity, which isn't awful, but you rarely actually need it. In this case you can useord
to convert each letter to its ordinal, then subtractord('a')
to get 0-25 instead of 97-122.This is particularly useful, because you can then use
chr
to convert back without needing a lookup.Note also that concatenating strings with
+=
isn't as fast as something likestr.join
.And since that
chr(((ord(letter) - ord('a') + key) % 26) + ord('a'))
is so ugly, I'd refactor that into a function.A point of maintainability: it's easier to write testably good code if you separate your inputs from your results. Right now you'd have to do some monkey patching of
stdin
to write unit tests for any of your functions, but if you move your requests for user input intomain
and out of their respective function, that gets much easier.In fact, when you consider the Caesar Cipher is reversible by flipping the sign of the key, you can simply do:
and not write a
decrypt
function at all!