1

In the first For Loop below, the code successfully shifts the letters num places to the right. The issue is in the second For Loop, it doesn't reverse it completely like it's supposed to. I've included output below. Any advice would be appreciated. Thanks

def cipher(message):
num = int(input())
final = []
finalReverse = []
for i in message:
        if i.isalpha():
                i = chr(ord(i)+num)
                final.append(i)
        if not i.isalpha():
                final.append(i)
final = ''.join(map(str, final))
print(final)
for i in final:
        if chr(ord(i)-num).isalpha:
                x = chr(ord(i)-num)
                finalReverse.append(x)
        if not chr(ord(i)).isalpha:
                finalReverse.append(char(ord(i)))
finalReverse = ''.join(map(str, finalReverse))
print(finalReverse)

cipher("The New York Times is in New York City.")

OUTPUT: (i entered number 3 for num)

1st For Loop's Output: Wkh Qhz \run Wlphv lv lq Qhz \run Flw||.

2nd For Loop's Output: TheNewYYorkTimesisinNewYYorkCityy+

Expected 2nd For Loop's Output: The New York Times is in New York City.

Lanie909
  • 849
  • 1
  • 8
  • 11
  • 1
    Your indentation seems inconsistent (sometimes 4 spaces and sometimes 8). That is a bit of a red flag and suggests that you are using both spaces and tabs. Hopefully you are not programming directly in IDLE's shell (as opposed to a code window). – John Coleman Feb 12 '18 at 15:30
  • Also -- your method of shifting occasionally shifts alphabetic plaintext to non-alphabetic cipher text. You need to implement some sort of wrap-around involving modular arithmetic so that e.g. `z` maps to a letter and not some character whose ascii value is beyond that of z – John Coleman Feb 12 '18 at 15:33

2 Answers2

1

There are several problems here.

You should be using else.

You follow if <condition>: with if not <condition>:. Instead, use else:

if i.isalpha():
    i = chr(ord(i)+num)
    final.append(i)
else:
    final.append(i)

This isn't just cosmetic... since you change i in the if block, it's possible for both the if and if not blocks to run in a single iteration. Using else solves this (and is better style anyway).

You forgot to call isalpha in the decrypt section

You need to add parens to the end of your isalpha calls in the decrypt section, otherwise only the if block will ever run (because a method or function object is always considered true).

if chr(ord(i)-num).isalpha():
    ...

You used the wrong variable in your decrypt else loop

You're appending x to the list in the decrypt section's else block, but x only makes sense in the if block. Use i.

if chr(ord(i)-num).isalpha():
    x = chr(ord(i)-num)
    finalReverse.append(x)
else:
    finalReverse.append(i)

Fixing those should produce your original message successfully.

glibdud
  • 7,550
  • 4
  • 27
  • 37
  • I noticed you edited your question while I was working on this, but as the indentation is no longer correct, I continued to answer the [original version](https://stackoverflow.com/revisions/48749806/1). – glibdud Feb 12 '18 at 16:07
  • Thank you so much! – Lanie909 Feb 12 '18 at 16:09
  • @Lanie909 It's also notable (but sort of off topic) that this doesn't perform a true caesar cipher, as encoding `xyz` with `num=3` should produce `abc`, not `{|}`. I'll leave that as an exercise for you to figure out. – glibdud Feb 12 '18 at 16:09
0

The answer is pretty simple. In first loop you didn't performed any conversion on spaces.

if not i.isalpha(): 
    final.append(i)

But in second loop you are trying to convert spaces before appending to decryption list.

if not chr(ord(i)-num).isalpha:           
    finalReverse.append(x)

Also you are again appending x which is the value from upper condition .

finalReverse.append(x)

Instead of this you just have to append original value.

if not chr(ord(i)).isalpha: 
    finalReverse.append(char(ord(i)))

Let me know if you still have an issue.

anurag0510
  • 763
  • 1
  • 8
  • 17
  • Thanks so much for the response. I'm afraid it's not working for me, I still get the same output. I updated my code in the question to what you said. Did I do it correctly? Thanks. – Lanie909 Feb 12 '18 at 16:01