A simple encryption program using Python

Question

I made this encryption program in Python.

I don't have much experience (or any experience at all) about encryption (since I just gave my 9th grade finals), but I had this idea about an algorithm some time back which would enable the user to encrypt words following an algorithm where the program would follow this process for each letter of the entered word; pseudocode:

Let the variable x be the position of the alphabet in the list of alphabets sorted alphabetically
Let the variable y be the position of the alphabet in the entered word
For example, if the user enters 'abcd', the program would find x and y for a, b, c and d one by one
Then, it would find the variable z = 26-x+y and z would be the position of the alphabet in the code 
 in the list of alphabets
In abcd : for a - x = 1, y = 1, so, z = 26-1+1 = 26, so coded alphabet = 26th alphabet = z
 Similarly, all of a,b,c and d will have coded alphabets as 'z'
 So, 'abcd' will be coded as 'zzzz'

Here's the Python code:

alphabets =['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']
# Defining all sorts of functions
def position(tofind,source):
 size = len(source)
 for i in range(0,size,1):
 if (source[i] == tofind):
 p = i + 1
 break
 return p
def encrypt(a):
 output = []
 sizea = len(a)
 for i in range(0,sizea,1):
 x = i+1
 y = position(a[i],alphabets)
 z = 26-x+y
 if (z>26):
 z = z % 26
 element = alphabets[z-1]
 output.append(element)
 return output
def converttolist(text):
 size = len(text)
 l = []
 for i in range(0,size,1):
 l.append(text[i])
 return l
# The main program
print ()
print ("NOTE : Please enter all the alphabets in lowercase...")
print ()
given = str(input("Please enter the word to be coded : "))
givenlist = converttolist(given)
outputlist = encrypt(givenlist)
print ()
print ("The code for ",given," is :-")
outputlistlength = len(outputlist)
for i in range(0,outputlistlength,1):
 print (outputlist[i],end = "")

Let me know what you think about it.

Answer 1

Going to run through this code making edits and explain as I go:

1. You only ever use position to find the position within the alphabet; I think it'd be clearer to just make this function's purpose more specific and call it something like index_in_alphabet.

2. Having narrowed and defined the purpose of this function, it can be implemented much more simply by subtracting the character values:

def index_in_alphabet(letter: str) -> int:
 """Converts a lowercase letter to an index from 1-26."""
 return 1 + ord(letter) - ord('a')

3. We probably also want it to raise an exception instead of returning an out-of-bounds value if letter isn't a lowercase letter. assert is an easy way to do that.

4. Similarly to how I used ord to replace alphabets for finding the index, you can use chr to replace it for generating the character from the index:

 element = chr(ord('a') + z - 1) # instead of alphabet[z-1]

5. Your entire converttolist function can be replaced with just:

def converttolist(text: str) -> List[str]:
 return list(text)

which of course in turn means that instead of converttolist() you can just use list().

6. Instead of making the caller convert the input to and from a list, you could just do it inside the function (so you accept a string and return a string). In fact, you don't need to convert anything to a list in the first place, because you can already index a string the same way you index a list!

7. Use if __name__ == '__main__': to indicate which part of your module is the "main program". This is the standard convention for Python and it has a practical purpose: if something else imports your module, whatever you put inside that if block won't get executed at import time (which is good).

8. The comment defining all sorts of functions isn't very helpful to the reader; a better use of comments is to explain what each function does!

9. Going to just kinda proofread some of the formatting here -- there are odd spaces and unnecessarily parentheses in some spots.

10. Eliminate unneeded variables!

Here's the code I ended up with:

def index_in_alphabet(letter: str) -> int:
 """Converts a lowercase letter to an index from 1-26."""
 index = 1 + ord(letter) - ord('a') 
 assert 1 <= index <= 26
 return index
def encrypt(a: str) -> str:
 """Returns the encrypted version of the input string."""
 output = ""
 for i in range(len(a)):
 x = i + 1
 y = index_in_alphabet(a[i])
 z = 26 - x + y
 if z > 26:
 z %= 26
 output += chr(z - 1 + ord('a'))
 return output
if __name__ == '__main__':
 print()
 print("NOTE : Please enter all the alphabets in lowercase...")
 print()
 given = str(input("Please enter the word to be coded: "))
 print()
 print("The code for", given, "is:", encrypt(given))

• Rajdeep Sindhu: Thanks, so, I basically gotta make my code shorter and more efficient, right?
• Samwise: Efficiency is also important, but most of my changes are about making the code easier to read, which is hugely valuable in the real world. The easier it is to read and understand your code, the easier it is to identify bugs, or to add new features to it. The best way to get better at writing code that's easy to read is to ask for reviews from other people, exactly like you did!
• infinitezero: I think the alphabet version is better since it's more flexible. Once the encryption goes beyond [a-z] your suggestion is harder to update.
• Samwise: Maybe! I feel like they teach you in school to make everything as "flexible" as possible, and for a long time I approached coding that way, but it's not necessarily always the right approach. I decided several years ago to fully embrace the principle of YAGNI and have no regrets. It's always easier to start simple and add complexity when needed than to start complex in the hope you'll have less work to do later.

Answer 2

I think it's a nice project. I would say that the main things for you to work on is getting further acquainted with Python's standard library and with standard practices, which is what most of my advice will be surrounding.

Minor improvements

For your alphabet, you could use ascii_lowercase from string, i.e.:

from string import ascii_lowercase
alphabet = [character for character in ascii_lowercase]

---

Unless I'm misreading, your function position() looks like an attempt at recreating list.index(value) (or in your case source.index(tofind)).

---

"Unneeded" variables can sometimes make sense if they improve readability, but your function:

def converttolist(text):
 size = len(text)
 l = []
 for i in range(0,size,1):
 l.append(text[i])
 return l

would be just as readable if written as:

def converttolist(text):
 l = []
 for i in range(0,len(text),1):
 l.append(text[i])
 return l

and while we're on that particular function, I would strongly recommend having a look at list comprehension---it's both faster and cleaner. Your function would then become:

def convert_to_list(text: str) -> list:
 return [text[i] for i in range(len(txt)]

but I should add that, for cases like this, even better is to just use in-line built-ins like str.split() or [character for character in text].

---

You don't need to write str(input(<whatever>)) since input already returns a string.

---

The function range() defaults to step-size 1, so writing range(start, end, 1) is unnecessary.

---

I would also recommend using a main function for your main loop. You could move all of the stuff in the bottom into a if __name__ == "__main__":, which also would allow you to load in this python script into other programs.

Naming

Remember that Readability counts. The standard in python is to use snake_case for variable names, but more importantly ensure that your names make the variables' purpose clear; avoid names like x and sizea.

Answer 3

A warning

As a toy this is fine, but please do not use it (or encourage others to use it) for real cryptographic application. It is fun as an exercise, but will not be sufficiently strong to protect you against certain common attacks.

Strings as sequences

In Python, a string is a sequence of one-character strings. So you don't need to represent it as a list of strings, because for your purposes that's what a string already is:

alphabets = 'abcdefghijklmnopqrstuvwxyz'

That said, you can replace the entire thing with string.ascii_lowercase:

from string import ascii_lowercase
alphabets = ascii_lowercase

Position

This whole function can be replaced with:

source.index(to_find)

Parens

We aren't in C/Java, so this:

if (z>26):

does not need parentheses.

Magic numbers

Do not hard-code 26 here:

z = z % 26

Instead, use len(alphabets). Also, use in-place modulus:

z %= len(alphabets)

• Reinderien: Thank you for the accept, but it might be a little hasty! I suggest that you wait for a day or so to see what other advice you get.
• Rajdeep Sindhu: Thanks a lot, a query though, the point that you made 'don't hard-code 26' was for flexibility, right? Because if I upgrade the program someday to include more characters such as periods or commas, then I won't have to change the number 26 every time. Got it, thanks again
• Reinderien: That's right. Generally, avoiding magic number is more maintainable.
• Schmuddi: Why do you recommend in-place operators?
• Reinderien: It's shorter and simpler.