-1

In my high school class I have been assigned the task of creating a keyword cipher. However when I run it through a python visualizer I still can not see what it is doing wrong.

Here is my code so far:

n = 0
l = 0
string = ""
message = input("enter your sentence: ")
message = (message.lower())
keyword = input("enter keyword: ")
keyword = (keyword.lower())
listkw = list(keyword)
def change(message,listkw,n,string,l):
    try:
        for letter in message:
            if letter == " ":
                string += " "
            else:
                temp = listkw[n]
                mtemp = ord(letter)
                mtemp -= 96
                ktemp = ord(temp)
                ktemp -= 96
                letterans = mtemp+ktemp
                if letterans >= 27:
                    letterans -= 26
                letterans += 96
                ans = chr(letterans)
                string += ans
                print (string)
                n+=1
                message = message[l:]
                l+=1
    except IndexError:
        n= 0
        change(message,listkw,n,string,l)
change(message,listkw,n,string,l)
print (string)

When I run it with the following input

enter your sentence: computingisfun enter keyword: gcse

it should print jrfubwbsnllkbq, because it gets the place in the alphabet for each letter adds them up and print that letter.

For example:

change('a', list('b'), 0, "", 0)

prints out c because a = 1 and b = 2 and a+b = 3 (which is (c))

But it prints out jrfupqzn, which is not at all what I expected.

Wayne Werner
  • 49,299
  • 29
  • 200
  • 290
A Tsang
  • 87
  • 9
  • 5
    What is your input? What is your expected output? What is your output instead? Please [edit] your question to include a [mcve] – Wayne Werner Jan 26 '17 at 18:37
  • I don't think it's a good idea to change the value of `message` while you're iterating over it. I also don't think it's a good idea to catch IndexErrors and just run the function a second time if something goes wrong. – Kevin Jan 26 '17 at 19:14

2 Answers2

0

I understand that you're in high school so I replace some piece of code that you write unnecessarily, you'll do better with experience ;)

First of all, you must know that it isn't a good idea programming based in an exception, is better if you add a condition and you reinitialize your n value so the exception it isn't necessary; n = n + 1 if n + 1 < len(listkw) else 0

Then, you have a little problem with the scope of the variables, you set string = "" at the start of your script but when call the function the string inner the function has a different scope so when you print(string) at the end you have an empty string value, so, the values that you use into the function like n, l and string it's better if you define inside the function scope and finally return the desired value (calculated (cipher) string)

So, the code it's something like this:

Read and initialize your required data:

message = input("enter your sentence: ").lower()
keyword = input("enter keyword: ").lower()
listkw = list(keyword)

Define your function:

def change(message,listkw):
    n = l = 0
    string = ""
    for letter in message:
        if letter == " ":
            string += " "
        else:
            temp = listkw[n]
            mtemp = ord(letter) - 96
            ktemp = ord(temp) - 96
            letterans = mtemp + ktemp
            if letterans >= 27:
                letterans -= 26
            letterans += 96
            string += chr(letterans)
            message = message[l:]
            l+=1
            n = n + 1 if n + 1 < len(listkw) else 0
    return string

Call and print the return value at the same time ;)

print(change(message,listkw))
Javier Aguila
  • 574
  • 4
  • 5
  • In some languages it's not a good idea, but it's totally fine to use exceptions in Python. (though the OPs usage is a bit suspect here) – Wayne Werner Jan 26 '17 at 20:15
  • @WayneWerner yeah! of course that it's a good practice use exceptions, I mean that in any language it's better if you control the flow with expected workflow and data, wait for an IndexError and call the function again isn't the best practice, of course it's better if you add exceptions to manage really errors like lost connections, timeout exceptions, and so on but not like a natural workflow – Javier Aguila Jan 26 '17 at 20:30
0

You've got quite a few problems with what you're doing here. You're mixing recursion, iteration, and exceptions in a bundle of don't do that.

I think you may have had a few ideas about what to do, and you started down one track and then changed to go down a different track. That's not surprising, given the fact that you're a beginner. But you should learn that it's a good idea to be consistent. And you can do this using recursion, iteration, slicing, or driven with exceptions. But combining them all without understanding why you're doing it is a problem.

Design

Let's unwind your application into what you actually are trying to do. Without writing any code, how would you describe the steps you're taking? This is what I would say:

For every letter in the message:

  • take the next letter from the keyword
  • combine the numeric value of the two letters
  • if the letter is beyond Z(ebra), start back at A and keep counting
  • when we reach the last letter in the keyword, loop back to the beginning

This gives us a hint as to how we could write this. Indeed the most straightforward way, and one that you've got partially done.

Iteratively

Here's another pointer - rather than starting of with a dynamic problem, let's make it pretty static:

 message = 'computing is awesome'

 for letter in message:
     print(letter)

You'll see that this prints out the message - one character per line. Great! We've got the first part of our problem done. Now the next step is to take letters from the key. Well, let's put a key in there. But how do we iterate over two strings at a time? If we search google for python iterate over two sequences, the very first result for me was How can I iterate through two lists in parallel?. Not bad. It tells us about the handy dandy zip function. If you want to learn about it you can search python3 zip or just run >>> help(zip) in your REPL.

So here's our code:

message = 'computing is awesome'
keyword = 'gcse'

for letter, key in zip(message, keyword):
    print(letter, key)

Now if we run this... uh oh!

c g
o c
m s
p e

Where's the rest of our string? It's stopping after we get to the end of the shortest string. If we look at the help for zip, we see:

continues until the shortest iterable in the argument sequence is exhausted

So it's only going to go until the shortest thing. Well that's a bummer. That means we need to have a key and message the same length, right? Or does it? What if our key is longer than the message? Hopefully by now you know that you can do something like this:

>>> 'ab'*10
'abababababababababab'

If we make sure that our key is at least as long as our message, that will work. So we can just multiply the key times the number of letters in our message. I mean, we'll have way more than we need, but that should work, right? Let's try it out:

message = 'computing is awesome'
keyword = 'gcse'*len(message)

for letter, key in zip(message, keyword):
    print(letter, key)

Sweet! It worked!

So now let's try just adding the ord values and let's see what we get:

for letter, key in zip(message, keyword):
    print(chr(ord(letter)+ord(key)))

Oh.. dear. Well those aren't ASCII letters. As you've already found out, you need to subtract 96 from each of those. As it turns out because math, you can actually just subtract 96*2 from the sum that we've already got.

for letter, key in zip(message, keyword):
    if letter == ' ':
        print()
    else:
        new_code = (ord(letter)+ord(key)-96*2)
        print(chr(new_code+96))

But we've still got non-alpha characters here. So if we make sure to just bring that value back around:

for letter, key in zip(message, keyword):
    if letter == ' ':
        print()
    else:
        new_code = (ord(letter)+ord(key)-96*2)
        if new_code > 26:
            new_code -= 26
        print(chr(new_code+96))

Now we're good. The only thing that we have left to do is combine our message into a string instead of print it out, and stick this code into a function. And then get our input from the user. We're also going to stick our key-length-increasing code into the function:

def change(message, keyword):
    if len(keyword) < len(message):
        keyword = keyword * len(message)
    result = ''
    for letter, key in zip(message, keyword):
        if letter == ' ':
            result += ' '
        else:
            new_code = (ord(letter)+ord(key)-96*2)
            if new_code > 26:
                new_code -= 26
            result += chr(new_code+96)
    return result

message = input('enter your sentence: ')
keyword = input('enter your keyword: ')
print(change(message, keyword))

Recursion

So we've got it working using iteration. What about recursion? You're definitely using recursion in your solution. Well, let's go back to the beginning, and figure out how to print out our message, letter by letter:

message = 'computing is awesome'

def change(message):
    if not message:
        return
    print(message[0])
    change(message[1:])

change(message)

That works. Now we want to add our key. As it turns out, we can actually do the same thing that we did before - just multiply it:

def change(message, keyword):
    if not message:
        return

    if len(keyword) < len(message):
        keyword = keyword*len(message)

    print(message[0], keyword[0])
    change(message[1:], keyword[1:])

Well that was surprisingly simple. Now let's print out the converted value:

def change(message, keyword):
    if not message:
        return
    if len(keyword) < len(message):
        keyword = keyword*len(message)
    new_code = (ord(message[0])+ord(keyword[0])-96*2)
    if new_code > 26:
        new_code -= 26
    print(chr(new_code+96))
    change(message[1:], keyword[1:])

Again we need to handle a space character:

def change(message, keyword):
    if not message:
        return

    if len(keyword) < len(message):
        keyword = keyword*len(message)

    if message[0] == ' ':
        print()
    else:
        new_code = (ord(message[0])+ord(keyword[0])-96*2)
        if new_code > 26:
            new_code -= 26
        print(chr(new_code+96))
    change(message[1:], keyword[1:])

Now the only thing that's left is to combine the result. In recursion you usually pass some kind of value around, and we're going to do that with our result:

def change(message, keyword, result=''):
    if not message:
        return result

    if len(keyword) < len(message):
        keyword = keyword*len(message)

    if message[0] == ' ':
        result += ' '
    else:
        new_code = (ord(message[0])+ord(keyword[0])-96*2)
        if new_code > 26:
            new_code -= 26
        result += chr(new_code+96)
    return change(message[1:], keyword[1:], result)

print(change(message, keyword))

Slicing

We used some slicing in our recursive approach. We even could have passed in the index, rather than slicing off parts of our string. But now we're going to slice and dice. It's going to be pretty similar to our recursive solution:

def change(message, keyword): if len(keyword) < len(message): keyword = keyword*len(message)

while message:
    print(message[0], keyword[0])
    message = message[1:]
    keyword = keyword[1:]

When you see that, it shouldn't be much of a stretch to realize that you can just put in the code from our recursive solution:

while message:
    if message[0] == ' ':
        print()
    else:
        new_code = (ord(message[0])+ord(keyword[0])-96*2)
        if new_code > 26:
            new_code -= 26
        print(chr(new_code+96))
    message = message[1:]
    keyword = keyword[1:]

And then we just combine the characters into the result:

def change(message, keyword):
    if len(keyword) < len(message):
        keyword = keyword*len(message)

    result = ''
    while message:
        if message[0] == ' ':
            result += ' '
        else:
            new_code = (ord(message[0])+ord(keyword[0])-96*2)
            if new_code > 26:
                new_code -= 26
            result += chr(new_code+96)
        message = message[1:]
        keyword = keyword[1:]
    return result

Further Reading

You can do some nicer things. Rather than the silly multiplication we did with the key, how about itertools.cycle?

What happens when you use modulo division instead of subtraction?

Community
  • 1
  • 1
Wayne Werner
  • 49,299
  • 29
  • 200
  • 290