Caesar cipher using the word length as key [closed]

Here are some high-level comments:

• As stated in the comments, indentation matters in Python, and as it stands your code wouldn’t compile.

• There isn’t much in the way of documentation or comments. Although this seems like a fairly trivial piece of code, it’s a good habit to get into – it makes code substantially easier to read, review and maintain.

  The one comment – about the purpose of importing string – is not particularly helpful, because the only use of string comes on the very next line.

• There’s a lot of repeated code, and nothing here can be reused. The encrypt and decrypt options differ by only one character.

  This is just begging to be broken into smaller functions.

• You should read PEP 8, the Python style guide. Among other things, you should look at line length and variable names.

Having skimmed the code, I’m going to dive in and try it. Let’s see what happens:

• If I make a mistake when entering my initial option, the script simply exits silently. That’s not a very good experience:

  $ python caesarlen.py
  Press 'e' to encrypt.
   Press 'd' to decrypt.
   press 'q' to quit.x
  Command not found. Please try again.
  Press 'e' to encrypt.
   Press 'd' to decrypt.
   press 'q' to quit.e
  

  Also, it would be nice if there was a newline and maybe a prompt character (say >) to help distinguish my input from the program’s output.

• Once I’ve chosen an option (encrypt or decrypt), I have to restart the script to change my mind. That’s not a very good flow.

  A good flow when testing encryption code is to encrypt a string, then decrypt the output to check you get the original string. Having to quit and restart the script is mildly annoying; it would be better it I could choose the action on every run.

• It would be good if there were some newlines sprinkled through the output to break it up a bit. Right now it’s all tight and hunched together:

  $ python caesarlen.py
  Press 'e' to encrypt.
   Press 'd' to decrypt.
   press 'q' to quit.d
  Type a sentence to be translated ('q' to quit).
  xlmw ku b amvbmvkm
  this is a sentence
  Type a sentence to be translated ('q' to quit).
  this is a sentence
  pdeo gq z kwflwfuw
  

  How about this:

  $ python caesarlen.py
  Press 'e' to encrypt.
  Press 'd' to decrypt.
  press 'q' to quit.
  > d
  
  Type a sentence to be translated ('q' to quit).
  > xlmw ku b amvbmvkm
  this is a sentence
  
  Type a sentence to be translated ('q' to quit).
  > this is a sentence
  pdeo gq z kwflwfuw
  

• Overall, the script seems pretty robust to user input. I’m unable to induce a traceback, exception or generally make it misbehave. 👍

Back to the code. Here’s some more detailed feedback:

• In quite a few places, you get the user’s raw_input and convert to lowercase. Wrap that in a function?

• Your encryption and decryption functions use this pattern:

  text_in = ""
  while text_in.lower() != 'q':
      text_in = get_string_from_user()
      if text_in == 'q':
          pass
  

  Rather than using a pass here, it would be better to use a break: that will explicitly break you out of the while loop without doing any more work. It also means you can simplify the while loop as follows, and save yourself from initialising the variable:

  while True:
      text_in = get_string_from_user()
      if text_in == 'q':
          break
  

  One other nitpick on the original while loop: you’re coercing all user input to lowercase, so checking text_in.lower() != 'q' is equivalent to checking text_in != 'q'. That simplification can be applied throughout.

• Your variable names could be better. Phrases like textIn and textOut are a very dry description; you should explain what these variables represent. Better alternatives would be something like plaintext and ciphertext.

• Be consistent about using print as a statement (line 48) vs a function (lines 25 and 44). I would suggest using it as a function, because it will make life easier when you move to Python 3.