A Caesar cipher in Python3

Question

This code basically just does a Caesar cipher on the contents of the variable "s" with a shift of 1.

Sorry if the comments on the code don't explain it well. I'm quite new to Python and I'm not very good at explaining.

Can this be code be improved?

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase

# Returns a list of each char in the given string.
def charSplit(str):
    return [char for char in str]


letters = charSplit(ascii_lowercase)
result = []
finalResult = ""
letterIndex = 0
strIndex = 0
s = "abcde test bla bla bla"
found = False

if __name__ == "__main__":
    while found == False:
        # Checks if the string has been converted to the cipher.
        if len(result) == len(s):
            found = True
        # Adds a space to the result variable if the string index is a space.
        elif s[strIndex] == " ":
            result.append(" ")
            strIndex += 1
        # Checks if the string index is equal to the letter index.
        elif charSplit(s)[strIndex].lower() == letters[letterIndex]:
            result.append(letters[letterIndex - 1])
            strIndex += 1
        # 
        else:
            if len(letters) - 1 == letterIndex:
                letterIndex = 0
            else:
                letterIndex += 1
    # Iterates through result and adds each string to the empty string, "finalResult".
    for l in result:
        finalResult += l

    print(finalResult)

Answer 1

You say you're new to Python. Well, that's ok. And a Caeser Cipher is a good place to start since it's complex enough for an absolute beginner while easy enough to understand what goes on, why and when.

One thing to keep in mind is that Python comes batteries included. This means a lot of what you want to accomplish is already written, one way or another. You just have to know where to find it.

For example, take the first function of your program:

def charSplit(str):
    return [char for char in str]

Simple enough. charSplit('foo') returns ['f', 'o', 'o'].

Did you know we can do the same with list? list('foo') returns ['f', 'o', 'o'] as well.

So, we're down 1 function. Let's create a new one. You're wrapping your code quite nicely here:

if __name__ == "__main__":

This makes sure the code is only running when the file itself is called, not when it's imported by another file. How about taking it a step further to clean things up?

def main():
    # all code that was previously behind the mentioned if-statement goes here

if __name__ == "__main__":
    main()

Everything above the if are global variables. I'm not a fan of global variables, so we'll get rid of them later. But let's assume for a moment you want to keep them around.

Since some of those are only read and never overwritten (pseudo-constants), it's usually preferred to CAPITALIZE them. Makes them easier to recognize.

How about another function? It may be a bit overkill here since the main function I wrote earlier already covers everything and the code only has one purpose, but code usually grows. Function-creep comes in and all of a sudden you have 300 lines of code in the same function. Better to split things up already, keeps things easier. Besides, it gives me an excuse to tell something about arguments.

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    # code here

s wasn't a particularly descriptive variable name, so that's inp now (input is already taken.) Can you see this growing? While we're at variable names, having both a result and finalResult is somewhat confusing. The entire construct is somewhat confusing.

# Iterates through result and adds each string to the empty string, "finalResult".
for l in result:
    finalResult += l

You're really just copying result here, so finalResult can go. Well, almost. We still need to go to a string, so we're interested in ''.join(result). That takes an empty string, and joins all parts of the result list into it.

I don't like the while loop. There's 3 reasons for this and I'm going to fix 2 now and 1 later. Why? To keep things simple and easy to follow.

It starts out like this:

while found == False:

Did you know you can rewrite that to make it even more obvious what's going on?

while not found:

Which, if you're familiar with other languages, reads like while(!found). Python has the not operator, which returns True if the expression behind it is False.

The other end of the loop can be improved as well. You're printing the result of the cipher, while we may want to do something completely different with it. Perhaps the result should be used in a different function? That's not the cipher's problem. Let main figure that out. All in all, the code so far would look like this with the above taken into account:

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase


LETTERS = list(ascii_lowercase)

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    found = False
    strIndex = 0
    letterIndex = 0
    result = []

    while not found:
        # Checks if the string has been converted to the cipher.
        if len(result) == len(inp):
            found = True
        # Adds a space to the result variable if the string index is a space.
        elif inp[strIndex] == " ":
            result.append(" ")
            strIndex += 1
        # Checks if the string index is equal to the letter index.
        elif list(inp)[strIndex].lower() == LETTERS[letterIndex]:
            result.append(LETTERS[letterIndex - 1])
            strIndex += 1
        # 
        else:
            if len(LETTERS) - 1 == letterIndex:
                letterIndex = 0
            else:
                letterIndex += 1

    return(''.join(result))

def main():
    print(caesar_cipher("abcde test bla bla bla", -1))

if __name__ == "__main__":
    main()

We haven't actually used shift yet, but we'll need it to fix another problem in your code: it's a very limited Caesar cipher. After all, a proper Caesar cipher takes a shift parameter. How far should the text be shifted? 1 character? 2? 13? We can make this go round and round and round. That's one of the good things about a Caesar cipher.

I'm still not a fan of the while loop here, so let's fix 2 things at a time. If our algorithm is correct, we can simply iterate over the input and know the output is correct at the end without keeping track of a found variable. And while we're at it, let's try to remove the other variables that are just to keep track as well.

You made a good start with importing ascii_lowercase, so let's build on that. Did you know string.ascii_lowercase (just like string.lowercase) has an index function? It provides us with the position of a character in a string.

What if we use that index to retrieve a different value of the alphabet instead? After all, an index is just a number. We could modify that number to get a different character altogether.

For every character in the input, if the character is a letter, get a different letter from the alphabet instead.

Oh, we can use a for loop instead of a while loop!

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase


LETTERS = list(ascii_lowercase)

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    result = []

    for character in inp:
        if character in LETTERS:
            # If it's a letter, we cipher it
            result.append(LETTERS[ascii_lowercase.index(character) + shift])
        else:
            # If it's NOT a letter, we won't cipher it
            result.append(character)

    return(''.join(result))

def main():
    print(caesar_cipher("abcde test bla bla bla", -1))

if __name__ == "__main__":
    main()

See how much easier that is to read? Now we just need a minor modification to account for the limited length of the alphabet. After all, what happens if someone wants a shift of 40? Or 51? Please welcome the % modulo operator. Divide by the length of the alphabet (26) and return the remainder.

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase


LETTERS = list(ascii_lowercase)

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    result = []

    for character in inp:
        if character in LETTERS:
            # If it's a letter, we cipher it
            result.append(LETTERS[(
                ascii_lowercase.index(character) + shift
                ) % len(LETTERS)])
        else:
            # If it's NOT a letter, we won't cipher it
            result.append(character)

    return(''.join(result))

def main():
    print(caesar_cipher("abcde test bla bla bla", 51))

if __name__ == "__main__":
    main()

Now, compare this to the original version. Can you see how we got here? You started out right, and now we got something even better. And this can probably be improved even further, I'm fairly sure we don't need result to be a list in the first place. Have fun figuring that out :-)

Answer 2

I hate to provide an "answer only" code review, but I'd like to expand upon Mast's "Python comes batteries included" point:

Python comes with a str.translate, function which will do a letter-for-letter substitution in a string:

>>> help(str.translate)
Help on method_descriptor:

translate(self, table, /)
    Replace each character in the string using the given translation table.

      table
        Translation table, which must be a mapping of Unicode ordinals to
        Unicode ordinals, strings, or None.

    The table must implement lookup/indexing via __getitem__, for instance a
    dictionary or list.  If this operation raises LookupError, the character is
    left untouched.  Characters mapped to None are deleted.

Continuing on the batteries included, it also has a str.maketrans function for creation of the translation table:

>>> help(str.maketrans)
Help on built-in function maketrans:

maketrans(x, y=None, z=None, /)
    Return a translation table usable for str.translate().

    If there is only one argument, it must be a dictionary mapping Unicode
    ordinals (integers) or characters to Unicode ordinals, strings or None.
    Character keys will be then converted to ordinals.
    If there are two arguments, they must be strings of equal length, and
    in the resulting dictionary, each character in x will be mapped to the
    character at the same position in y. If there is a third argument, it
    must be a string, whose characters will be mapped to None in the result.

So, to continue improvements upon Mast's solution:

import string

def caesar_cipher(msg, shift, alphabet=string.ascii_lowercase):
    shift %= len(alphabet)
    xlate = str.maketrans(alphabet, alphabet[shift:] + alphabet[:shift])
    return msg.translate(xlate)

def main():
    print(caesar_cipher("abcde test bla bla bla", 51))

if __name__ == "__main__":
    main()

Answer 3

An super efficient way to do this would be to use list comprehension

You ask the user to input the cesar shift and message

Then using list-comprehension you build a list of the new letters.

Then you use the "".join() method to convert back to a string

def cesar_encode(message, shift):
    new_message = [chr(ord(char) + shift) if (ord(char) + shift) < 122 else chr(47+shift+122-ord(char)) for char in message]
    return "".join(new_message)

if __name__ == "__main__":
    shift = input("Enter cesar shift: ")
    message = input("Enter the message to encode: ")
    print (cesar_encode(message, int(shift)))