-4

I'm writing a Caesar Cipher code for a part of a controlled assessment. I built a fully functioning program and I thought I had it sussed but after changing a few things around I went to check back and everything has gone wrong!

The code's quite untidy but I'm getting a bit sick of coding this now and have taken to the internet to get someone else's view.

Code:

answer ="C"
while answer == "C":

    lettersList=['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 menu():
        userChoice=input("Would you like to encrypt or decrypt a message? E or D.\n").lower()
        while userChoice != "e" and userChoice != "d":
            print("Invalid.")
            userChoice=input("Would you like to encrypt or decrypt a message? E or D.\n").lower()
        print("\n")
        return userChoice

    def getPlaintext():
        plaintext= input("Please enter the message you would like encrypted/decrypted\n").lower()
        while plaintext.isalpha() == False:
            print("Invalid")
            plaintext=input("Please enter the message you would like encrypted/decrypted\n").lower()
        print("\n")
        return plaintext

    def getKey():
        key=int(input("Please enter a key. 1-26\n"))
        while key > 26 or key < 1:
            print("Invalid.")
            key=int(input("Please enter a key. 1-26\n"))
        print("\n")
        return key


    def encryptText(plaintext,key):
        characterNumber = 0
        newMessage = ""
        for characters in plaintext:
            character = plaintext[characterNumber]
            characterPosition = lettersList.index(character)
            newPosition=character+key
            newLetter = lettersList[newPosition]
            newMessage = (newMessage+newLetter)
            characterNumber= characterNumber+1
        print(newMessage)

    def decryptText(plaintext,key):
        characterNumber = 0
        newMessage = ""
        for characters in plaintext:
            character = plaintext[characterNumber]
            characterPosition = lettersList.index(character)
            print(characterPosition)
            newPosition=characterPosition-key
            newLetter = lettersList[newPosition]
            newMessage = (newMessage+newLetter)
            characterNumber= characterNumber+1
            newMessage = (newMessage.lower())
        print(newMessage)

    userChoice=menu()
    plaintext=getPlaintext()
    key=getKey()
    if userChoice == "e":
        encryptText(plaintext,key)
    elif userChoice == "d":
        decryptText(plaintext,key)
    print(newMessage)
Makoto
  • 104,088
  • 27
  • 192
  • 230
  • 1
    You need to tell us how the code currently behaves and what you expect from it. *Doesn't work* is not at all useful. You can edit your question to include this information – Artjom B. Mar 08 '15 at 20:27
  • I've rolled this back. Don't edit pertinent parts of the question because your problem is solved. – Makoto Mar 08 '15 at 22:05

2 Answers2

0

In decypryptText, newPosition can be negative. You should wrap around the whole alphabet in this case. The best way to do that is to use the % operator:

newPosition = (characterPosition - key) % len(lettersList)

By the way, using this in encryptText too lets you make lettersList only one copy of the alphabet, instead of the two copies you use.

There are other things that can be polished in this code, but they are not as important as this one.

Edit: if you want it capitalized for decryptText, you can make decryptText just return newMessage.upper(). In the same way, you can make encryptText return newMessage.lower().

matiasg
  • 1,927
  • 2
  • 24
  • 37
0

You've got some simple misspellings in your code which prohibit the correct functioning. for characters in plaintext: should be for character in plaintext: - in your code you create a new variable characters which is not used anywhere. The correct encrypting function with comments looks like this:

def encryptText(plaintext, key):
    newMessage = ""
    for character in plaintext:  # 'character' holds each letter from 'plaintext'
        characterPosition = lettersList.index(character)
        newPosition = characterPosition + key  # shift the index, not the character!
        newLetter = lettersList[newPosition]
        newMessage = newMessage + newLetter  # append new letter to new message

    print(newMessage)

To make this work correctly on lettersList with just one occurrence of each character (i.e. of length 26) you need to check that the shifted index is not out of bounds; that is, the character is not greater than 26. If it is, then subtract 26, like this

newPosition = characterPosition + key
if newPosition >= len(lettersList) then:
    newPosition -= len(lettersList)

or use the modulo operator as suggested. This should get you the idea how to modify the decrypt function to "wrap around" as well.

user1016274
  • 4,071
  • 1
  • 23
  • 19
  • Hi, even with this it does not work. When it runs I get 'NameError: name 'newMessage' is not defined' when I clearly define it? – Jack Evans Mar 08 '15 at 21:28
  • You define newMessage inside the encryptText or decryptText scope, but the last line in your script does not get that newMessage. You can return newMessage in both methods and then use it as newMessage = encryptText(paintext, key). – matiasg Mar 08 '15 at 21:36
  • by the way, I don't think that the characters/caracter issue is the main problem here. Yes, it is confusing and not the best way to loop over the plaintext string, but it's not a bug, as you are incrementing characterNumber. – matiasg Mar 08 '15 at 21:38
  • okay, finally. Sorry, before it was just problem after problem but the program is now all working well. Any idea's how to capitalise the string if its come from the encryptText() and leave it lower case if from decryptText()? It's refusing to do it at the moment.. – Jack Evans Mar 08 '15 at 21:39
  • @Makoto: thanks for editing, I will use the correct formatting in the future. But I had to rephrase my remark about the lettersList used to make clear that I really meant "one alphabeth" and not "one character". Hope it's more clear now. – user1016274 Mar 08 '15 at 22:19
  • That's fine. I used "character" since "alphabeth" isn't a word in English, and I'm glad to see that you've since corrected it. – Makoto Mar 08 '15 at 22:20