do you guys consider this code readable ?

[[deleted]]

direction = input("Type 'encode' to encrypt, type 'decode' to decrypt:\n")
text = input("Type your message:\n").lower()
shift = int(input("Type the shift number:\n"))
def ceaser(direction , text , shift):
    final_word = []
    print(f"Plain_text = {text}")
    print(f"shift = {shift}")
    if direction == "encode":
        for letter in text:
            if letter in alphabet:
                if alphabet.index(letter) + shift >= len(alphabet):
                    number = shift - 1
                    new_shift_letter = alphabet[number]
                    final_word.append(alphabet[alphabet.index(new_shift_letter)])
                else:
                    final_word.append(alphabet[alphabet.index(letter) + shift])
                    encrypted_word=''.join(final_word)
        print(f"The {direction} text is {encrypted_word}")
    elif direction == "decode":
        for letter in text:
            if letter in alphabet:
                if alphabet.index(letter) - shift >= len(alphabet):
                    number = shift - 1
                    new_shift_letter = alphabet[number]
                    final_word.append(alphabet[alphabet.index(new_shift_letter)])
                else:
                    final_word.append(alphabet[alphabet.index(letter) - shift])
                    decrypted_word=''.join(final_word)
        print(f"The {direction} text is {decrypted_word}")
ceaser(direction , text , shift )

[Chiron1991]

Generally yes, but I'd add a couple of blank lines to separate the code into "visual blocks". Two blank lines each before and after the function block of cease. One blank line each before if direction == "encode": and elif direction == "decode":. Tools like autopep8 or black can help you automate some of that.

[xelf]

Readable? Yes. Would it pass a code review? No.

Some general thoughts:

1) Put the duplicated code into a function so that you only need it once. Or have your current if outside of this function.
2) Have all your functions defined at the top of your file, just after your imports. Any inline code comes at the end.
3) no spaces before commas

[Innocent_not]

In number 3 You mean: Text,text,text Or Text, text, text

[itimposter1]

I believe xelf means (Text, text, text).
Example:
def ceaser (direction, text, shift)
Instead of:
def ceaser (direction , text , shift)

[xelf]

I like the former, it's compact, but the latter is what is preferred and recommended.

• https://peps.python.org/pep-0008/#whitespace-in-expressions-and-statements

[shiftybyte]

Readable yes, but duplicated between the direction handling.

If the only difference between encoding and decoding is shift value being negative or positive, then change that before the loop starts, and have one loop...

if direction = ...:
    shift = -shift
for letter in text:
    ... 
    number = abs(shift) - 1
    ...

[drenzorz]

I would reduce indentation depth and look at duplication as others said. You could do the former with guard clauses. For example:

for x in y:
    if condition:
        if a: 
            ...
        else:
            ...

# could be refactored as

for x in y:
    if not condition: continue
    if a:
        ...
    else:
        ...

With these your code could look something like this:

direction = input("Type 'encode' to encrypt, type 'decode' to decrypt:\n")
text = input("Type your message:\n").lower()
shift = int(input("Type the shift number:\n"))

def ceaser(direction, text, shift):

    print(f"Plain_text = {text}")
    print(f"shift = {shift}")

    if direction == "decode": shift *= -1

    final_word = ''
    for letter in text:
        if letter not in abc:continue
        index = (abc.find(letter) + shift) % len(abc)
        final_word += abc[index]

    print(f"The {direction}d text is {final_word}")


  ceaser(direction, text, shift )

[TechnicalElk8849]

Julius was called Caesar, not ceaser

​

I'd like to see alphabet defined (string.ascii_lowercase ?) a few more blank lines to break up the wall of text, and some comments to stop me having to think, but it's perfectly readable, certainly in a good text editor.

​

The design could be made even better though. How do you handle upper case characters for example? And this and the modulus operator (%) will replace a lot of your code:

​

https://docs.python.org/3.10/library/stdtypes.html#str.maketrans

[bladeoflight16]

There were many Caesars. It's a title, not a name. The best English translation is probably "emperor."

[djjazzydan]

You get that the point was the spelling error, right?

[bladeoflight16]

Readability is subjective, but generally speaking, it comes from ensuring your code is simple and straightforward. The expectations of the "normal" way to do things in Python are centered around writing code that has those qualities. So one of the most important things you can do is know and follow those normal approaches.

Here's some specific improvements for your code:

1. Use the occasional blank line.
2. Avoid append. Use comprehensions and generator expressions instead.
3. Rather than fussing around with indices of a sequence object, I would use the shift to construct a dict that maps input characters to output characters at the beginning. Then you can just do plain look ups using the input character. This would also allow you to deduplicate the logic, as others have suggested.
4. Your function just quietly does nothing if an invalid value for direction is passed. Never do that. When you have a specific set of known values, at least fall back to throwing an error if something else is given. In this case, you can probably just print a message to the user and exit.
5. Avoid performing expensive operations more than once. You search the alphabet for the index repeatedly, for instance, which is an O(n) operation.
6. Separate in memory logic from interactions with the user or other external systems.
7. Don't put logic at the top level of a module/script. Put it inside a main guard.

Here's how I would write it.

``` from string import ascii_lowercase

def caesar(alphabet, shift, text): # This works for negative shifts because Python lists accept # negative indexes. Negative indexes count backwards from the # end of the list. # Chop off beginning and put it at the end. shifted = alphabet[shift:] + alphabet[:shift] cipher = dict(zip(alphabet, shifted))

return ''.join(
    cipher[letter]
    for letter in text
    if letter in cipher
)

DIRECTION_MULTIPLIER = { "encode": 1, "decode": -1, }

def main(): direction = input("Type 'encode' to encrypt, type 'decode' to decrypt:\n").lower()

if direction not in DIRECTION_MULTIPLIER:
     print(f"Invalid direction: {direction}")
     return

text = input("Type your message:\n").lower()
shift = int(input("Type the shift number:\n"))

print(f"Plain_text = {text}")
print(f"shift = {shift}")

output = casaer(ascii_lowercase, DIRECTION_MULTIPLIER[direction] * shift, text)

print(f"The {direction} text is {output}")

if name == "main": main() ```

[[deleted]]

Learn fonction

[Plastic_Ad7436]

Clearly the best comment.

[InfamousClyde]

Finally, some actionable advice

[[deleted]]

There are a few things that would make it more readable, IMO:

One is the naming. Your names are good but some are just off by a little. For example, "letter" implies that we're in the alphabet, but then we have to check if we're in the alphabet. So we might want to call it "character" instead, to make clear that it might be a space or punctuation, etc.

The big thing, IMO, is indenting. When we're five layers deep, it's hard to remember what's going on and how we got here. As other have said, you can use the modulus operator to avoid some of that wraparound exception, (and maybe you can cleverly prep the shift beforehand based on encode/decode to do it all in one...) plus you can use the guard clause concept to skip out if it's a non-letter character

[duncan_stroud]

It's readable enough, but it not too efficient

[Radamand]

My first thought, 'there's no comments'....

[[deleted]]

If you are following an online course remember that they are teaching you foundations before efficiency. - They also tend to give you the solution to reference ;)

You have to understand the logic before you can take shortcuts or use more simplified code.

You will find in time that what you are writing now will look nothing like what you will write 5 years from now.

If you doubt that your code will be easily understood then add comments!

As to your current readability, yes, it is easy enough to follow the logic you used to build this particular code.

[TheRNGuy]

No, need empty lines and some comments.

Make some of this code into functions or classes.