pythonif-statementwhile-loopcaesar-cipher

Why does my while loop at the bottom run once?


Caesar Cipher

alphabet = ['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', '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']

def caesar(text, shift, direction):
  cipher_text = ""
  for letter in text:
    text_position = alphabet.index(letter)
    if direction == "encode":
      new_text_position = text_position + shift
    elif direction == "decode":
     new_text_position = text_position - shift
    else:
      print("Error, invalid entry.")
    cipher_text += alphabet[new_text_position]
  print(f"Your cipher text is {cipher_text}")

direction = input("Type 'encode' to encrypt, or 'decode' to decrypt:\n")
while direction != "encode" or direction != "decode":
  direction = input("Invalid entry. Type 'encode' to encrypt, or 'decode' to decrypt:\n")
  if direction == "encode" or direction == "decode":
    text = input("Type your message:\n").lower()
    shift = int(input("Type the shift number:\n"))
    caesar(text, shift, direction)

This is a Caesar Cipher program from Angela Yu's Udemy course on Python. The idea at the bottom is simply to allow the user to progress as long as the input is 'encode' or 'decode'. As you can see, the while loop runs once even if the input is a valid answer. It should just skip right to the 'if' statement.

I tried to change the logic of the 'direction' variable from 'or' to 'and' as per someone's suggestion on the technology board on 4chan, but that doesn't work because it only accepts one input. So then I tried changing the while to an if, which works, but doesn't allow the user to repeat in the event that the input is not one of the two options. I also tried to move the if statement out of the domain of the while loop, but that just causes the program to run the while loop even if the correct answer is inputted.


Solution

  • Note: It is better to attach your code directly instead of uploading a screenshot, so that anyone interested in answering can run/debug it.

    The logic error is caused by 2 mistakes.

    Firstly, the condition for that while loop is such that it will always be True because:

    1. If you enter "encode": direction != "encode" will be False but direction != "decode" will be True and hence the or condition will be True.
    2. If you enter "decode": direction != "decode" will be False but direction != "encode" will be True and hence the or condition will be True.

    Therefore, you must use the and operator so that the loop is executed only if direction is not equal to both "encode" and "decode".

    Secondly, even if you replace or with and, the error will persist because you placed the last if statement within that while loop by keeping the same level of indent. This means that the if statement will only be executed if the while loop is entered. Otherwise, the interpreter will go over the whole while loop, which includes the if statement. Therefore you have to un-indent that if statement so that it is separate from that while loop.

    It should look like this:

    direction = input("Type 'encode' to encrypt, or 'decode' to decrypt:\n")
    while direction != "encode" and direction != "decode":
        direction = input("Invalid entry. Type 'encode' to encrypt, or 'decode' to decrypt:\n")
    
    if direction == "encode" or direction == "decode":
        text = input("Type your message:\n").lower()
        shift = int(input("Type the shift number:\n"))
        caesar(text, shift, direction)
    

    Also, the while loop looks better/more pythonic like this:

    while True:
    direction = input("Type 'encode' to encrypt, or 'decode' to decrypt:\n")
    if direction != "encode" and direction != "decode":
        print("Invalid input. ", end="")
        continue
    else:
        break
    

    The loop is infinite by default, because the actual condition is replaced with True and will remain so forever, which means that I will have to do something else to "break" out of the loop as there is no condition for the while loop to check for. If, after taking user input, the direction is anything other than "encode" or "decode", it prints "Invalid input. " and the end="" argument specifies what to "end" the string with. It is end=\n by default which is why you always get a newline with a print function. So now, the following print function's string is printed right next to this string as they are on the same line. If the conditions are met, however, it follows the else clause and "breaks" out of the loop.

    Finally, that last if statement is unnecessary because the while loop is going to take care of the input validation. Your final code should look something like this:

    alphabet = ['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', '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']
    
    
    def caesar(text, shift, direction):
        cipher_text = ""
        for letter in text:
            text_position = alphabet.index(letter)
            if direction == "encode":
                new_text_position = text_position + shift
            elif direction == "decode":
                new_text_position = text_position - shift
            else:
                print("Error, invalid entry")
            cipher_text += alphabet[new_text_position]
        print(f"Your cipher text is {cipher_text}")
    
    
    while True:
        direction = input("Type 'encode' to encrypt, or 'decode' to decrypt:\n")
        if direction != "encode" and direction != "decode":
            print("Invalid input. ", end="")
            continue
        else:
            break
    
    
    if direction == "encode" or direction == "decode":
        text = input("Type your message:\n").lower()
        shift = int(input("Type the shift number:\n"))
        caesar(text, shift, direction)