I wrote this looping Caesar cipher with a default key. I'd love for some input on how I could make this operate better/more effectively.
#Et Tu, Brute? [Caesar's Cipher]
#loops?
#Determine type of cipher...
doLoop=("y")
while doLoop==("y"):
#yep. loops.
det=input("Enter N for number or C for character; 'exit' to quit program ").lower() #lower case for simplicity! it don't care!
#Now the fun part!
#Numbers...
if (det)==("n"):
print("The default cipher key is 5")
numRaw=input("Enter number to be decoded: ")
if ord(numRaw)<ord("5"):
numDec=ord(numRaw)+5
else:
numDec=ord(numRaw)-5
print("The decoded number is: ",(chr(numDec)))
#Letters...
if (det)==("c"):
print("The default cipher key is 13")
charRaw=input("Enter character to be decoded: ").lower() #lower case for simplicity!
if ord(charRaw)<ord("m"):
charDec=ord(charRaw)+13
else:
charDec=ord(charRaw)-13
print("The decoded character is: " , (chr(charDec)))
if det==("exit"):
break
3 Answers 3
Python strings have a translate
method that applies a substitution cipher. There is also a function str.maketrans
(string.maketrans
in Python 2) that helps with building the translation table:
>>> from string import ascii_lowercase as alphabet
>>> shift = 13
>>> cipher = str.maketrans(alphabet, alphabet[shift:] + alphabet[:shift])
>>> print("caeser salad is tasty".translate(cipher))
pnrfre fnynq vf gnfgl
of coarse in python2(with a shift of 13) one can simply do "My Message".encode("rot13")
Register all the Caesar cyphers as codecs. Codecs are actually quite easy to make – they consist of a function that takes an encoding name and returns either None
if it doesn't know that codec name or a CodecInfo
object if it does. In fact, rot_13
is in the Python library in Python 2, so you can use that as a model.
import codecs
def search_function(encoding_name):
try:
name, rot = encoding_name.split('-')
rot = int(rot, 10)
if name != 'rot': return None
except ValueError:
return None
rot = rot % 26
offsets = [(i - rot) % 26 for i in range(26)]
decoding_map = codecs.make_identity_dict(range(256))
decoding_map.update((i, 65 + j) for i, j in enumerate(offsets, 65))
decoding_map.update((i, 97 + j) for i, j in enumerate(offsets, 97))
encoding_map = codecs.make_encoding_map(decoding_map)
class Codec(codecs.Codec):
def encode(self,input,errors='strict'):
return codecs.charmap_encode(input,errors,encoding_map)
def decode(self,input,errors='strict'):
return codecs.charmap_decode(input,errors,decoding_map)
class IncrementalEncoder(codecs.IncrementalEncoder):
def encode(self, input, final=False):
return codecs.charmap_encode(input,self.errors,encoding_map)[0]
class IncrementalDecoder(codecs.IncrementalDecoder):
def decode(self, input, final=False):
return codecs.charmap_decode(input,self.errors,decoding_map)[0]
class StreamWriter(Codec,codecs.StreamWriter):
pass
class StreamReader(Codec,codecs.StreamReader):
pass
return codecs.CodecInfo(
name='rot-{}'.format(rot),
encode=Codec().encode,
decode=Codec().decode,
incrementalencoder=IncrementalEncoder,
incrementaldecoder=IncrementalDecoder,
streamwriter=StreamWriter,
streamreader=StreamReader,
)
codecs.register(search_function)
>>> for i in range(26): print letters.encode('rot-{}'.format(i))
...
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ
bcdefghijklmnopqrstuvwxyzaBCDEFGHIJKLMNOPQRSTUVWXYZA
cdefghijklmnopqrstuvwxyzabCDEFGHIJKLMNOPQRSTUVWXYZAB
defghijklmnopqrstuvwxyzabcDEFGHIJKLMNOPQRSTUVWXYZABC
efghijklmnopqrstuvwxyzabcdEFGHIJKLMNOPQRSTUVWXYZABCD
fghijklmnopqrstuvwxyzabcdeFGHIJKLMNOPQRSTUVWXYZABCDE
ghijklmnopqrstuvwxyzabcdefGHIJKLMNOPQRSTUVWXYZABCDEF
hijklmnopqrstuvwxyzabcdefgHIJKLMNOPQRSTUVWXYZABCDEFG
ijklmnopqrstuvwxyzabcdefghIJKLMNOPQRSTUVWXYZABCDEFGH
jklmnopqrstuvwxyzabcdefghiJKLMNOPQRSTUVWXYZABCDEFGHI
klmnopqrstuvwxyzabcdefghijKLMNOPQRSTUVWXYZABCDEFGHIJ
lmnopqrstuvwxyzabcdefghijkLMNOPQRSTUVWXYZABCDEFGHIJK
mnopqrstuvwxyzabcdefghijklMNOPQRSTUVWXYZABCDEFGHIJKL
nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM
opqrstuvwxyzabcdefghijklmnOPQRSTUVWXYZABCDEFGHIJKLMN
pqrstuvwxyzabcdefghijklmnoPQRSTUVWXYZABCDEFGHIJKLMNO
qrstuvwxyzabcdefghijklmnopQRSTUVWXYZABCDEFGHIJKLMNOP
rstuvwxyzabcdefghijklmnopqRSTUVWXYZABCDEFGHIJKLMNOPQ
stuvwxyzabcdefghijklmnopqrSTUVWXYZABCDEFGHIJKLMNOPQR
tuvwxyzabcdefghijklmnopqrsTUVWXYZABCDEFGHIJKLMNOPQRS
uvwxyzabcdefghijklmnopqrstUVWXYZABCDEFGHIJKLMNOPQRST
vwxyzabcdefghijklmnopqrstuVWXYZABCDEFGHIJKLMNOPQRSTU
wxyzabcdefghijklmnopqrstuvWXYZABCDEFGHIJKLMNOPQRSTUV
xyzabcdefghijklmnopqrstuvwXYZABCDEFGHIJKLMNOPQRSTUVW
yzabcdefghijklmnopqrstuvwxYZABCDEFGHIJKLMNOPQRSTUVWX
zabcdefghijklmnopqrstuvwxyZABCDEFGHIJKLMNOPQRSTUVWXY
-
1\$\begingroup\$ what is the value of using a codec rather than doing this some simpler way? \$\endgroup\$Stuart– Stuart2013年10月15日 17:40:16 +00:00Commented Oct 15, 2013 at 17:40
-
2\$\begingroup\$ @Stuart what's the value of a Caesar cypher period? \$\endgroup\$kojiro– kojiro2013年10月15日 18:15:38 +00:00Commented Oct 15, 2013 at 18:15
-
\$\begingroup\$ @Stuart Anyway, there's a lot of power in codecs. They can be used to transform bytestrings, including just about any stream of data that needs transforming, and they are bidirectional. I can decode any Caesar cypher as easily as it was encoded.
'foo'.encode('rot-9').decode('rot-9') vs 'foo'.translate(maketrans(forward_shift)).translate(maketrans(backward_shift))
\$\endgroup\$kojiro– kojiro2013年10月15日 18:36:12 +00:00Commented Oct 15, 2013 at 18:36
Style
According to PEP8 you should:
- Limit your lines to <80 characters
- put spaces around your operators
- privilege underscore_names over camelCase for variables
There are trailing spaces on line 6, certain projects will refuse your code based on that.
You don't need parenthesis around values. ("y")
is equivalent to "y"
, (det)
to det
, etc.
I would use more newlines to separate different parts of the code.
Bad comments
The number one thing your code is lacking is proper commenting. The comments you give aren't very informative. In this case it doesn't matter much because the code is very easy to grasp, but practicing writing good and informative comments will definitely pay off. Also, you should (ab)use doctests.
Comparing chars
You don't need to compare the ord
of characters. Comparing strings is always alphabetical:
>>> "a" < "b"
True
>>> "b" < "a"
False
Useless lines
doLoop=("y")
while doLoop==("y"):
The variable doLoop
is never used.
Also, If you planned on using it as a flag to get out of the loop, a boolean would make more sense than a string.
Repeated code
You're repeating the same logic twice. Why not make a function out of it and limit code duplication?
(det)==("c")
=>det == "c"
\$\endgroup\$<=ord('m')
, not<
. Otherwise, you mapm
to a backtick instead of toz
(which also means your cipher isn't reversible). Also, it's simpler to just use the%
operator to wrap aroundz
instead of trying to get the fiddly details right and introducing bugs like this one. \$\endgroup\$