3
\$\begingroup\$

Encryption is all the rage nowadays but let us not forget what it was like back before the advent of internet. I have written a small, simple cipher that takes texts, gets their char value, and adds or subtracts a certian given number from them aka "shifting" the keys.

If the receiving end has the direction and then number of the shift, he or she can easily decrypt the code. It's not a real cipher, and nothing important at all. Just something I've written as practice:

 """ This is a simple alphabetical shift encypter. Enter -1 or 1 for the direction, your desired shift and laslty, the
    text to cipher.
    Written by Sami Dena ([email protected])
"""

def cipher(direction, shift, text):
    text_split = list(text)

    text_split = [ord(i) for i in text_split]

    text_shift = [str(i + (direction * shift)) for i in text_split]

    return str.join(',', text_shift)


def decipher(direction, shift, text):
    text_split = str.split(text, ',')

    text_shift = [chr(int(i) - (direction * shift)) for i in text_split]

    return str.join('', text_shift)


if __name__ == '__main__':
    string = "The encryption step performed by a Caesar cipher is often" \
             " incorporated as part of more complex schemes, such as the Vigenère cipher" \
             ", and still has modern application in the ROT13 system. As with all single-alphabet" \
             " substitution ciphers, the Caesar cipher is easily broken and in modern practice offers " \
             "essentially no communication security."

    encrypted_string = cipher(-1, 4, string)
    print(encrypted_string)
    print(decipher(-1, 4, encrypted_string))

Thanks a lot for your reviews.

\$\endgroup\$
5
  • \$\begingroup\$ Did you mean to leave your name and email address in the documentation at the top? \$\endgroup\$
    – Phrancis
    Aug 18, 2016 at 19:32
  • \$\begingroup\$ @Phrancis: Is it against the rules? \$\endgroup\$ Aug 18, 2016 at 19:58
  • 3
    \$\begingroup\$ No it's not, I was just making sure that was OK with you to put that out in public, sometimes it is done by accident. Hope you get some great answers! \$\endgroup\$
    – Phrancis
    Aug 18, 2016 at 20:00
  • 1
    \$\begingroup\$ Hrmmm; that is not a real Ceasar cipher though. If I would cipher 'z' with direction 1 and shift 2, I expect a 'b'. As a result, this code would crash for high shift values or low or high chars. (err... actually not, because you give a comma seperated list of int back) \$\endgroup\$
    – Sumurai8
    Aug 18, 2016 at 20:54
  • \$\begingroup\$ @Sumurai8: It's not ceasar's cipher, it's my own cipher. \$\endgroup\$ Aug 19, 2016 at 0:54

1 Answer 1

2
\$\begingroup\$

You know what comprehensions are, which is good!
But you've a couple of problems:

  • str.join(',', text_shift) is abnormal and should probably be ','.join(text_shift).
    str.split(text, ',') is equally abnormal and should be changed to text.split(',').

  • text_split = list(text) is unneeded, as you can loop thought the string the exact same way.

  • text_split = [ord(i) for i in text_split] can be merged into text_shifts comprehension.

  • You can change both cipher and decipher to be one-liners.

I'd also change both direction * shift to be a variable, just to ease reading of the comprehension slightly.

And so you should be able to end up with functions that look like:

def cipher(direction, shift, text):
    order = direction * shift
    return ','.join([str(ord(i) + order) for i in text])


def decipher(direction, shift, text):
    order = direction * shift
    return ''.join([chr(int(i) - order) for i in text.split(',')])

Finally, your module level doc-string should have a short description on the first line, and then optionally have two spaces and a longer description.

You used implicit string concatenation, but you kept using \ at the end of the line. Instead I'd change you string to be wrapped in () to remove these.

I also disliked that you added your whitespace characters to the front of the next string. It looks odd, and is easier to read/ignore when it's on the other line.

And so in all I'd end up with:

"""
Simple alphabetical cipher

This is a simple alphabetical shift encypter. Enter -1 or 1 for the direction,
your desired shift and laslty, the text to cipher.
   Written by Sami Dena ([email protected])
"""

def cipher(direction, shift, text):
    order = direction * shift
    return ','.join([str(ord(i) + order) for i in text])


def decipher(direction, shift, text):
    order = direction * shift
    return ''.join([chr(int(i) - order) for i in text.split(',')])


if __name__ == '__main__':
   string = ("The encryption step performed by a Caesar cipher is often "
             "incorporated as part of more complex schemes, such as the Vigenère cipher, "
             "and still has modern application in the ROT13 system. As with all single-alphabet "
             "substitution ciphers, the Caesar cipher is easily broken and in modern practice offers "
             "essentially no communication security.")

   encrypted_string = cipher(-1, 4, string)
   print(encrypted_string)
   print(decipher(-1, 4, encrypted_string))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.