3
\$\begingroup\$

I know there are plenty of Caesar Cipher reviews on here, but I especially wanted to see how other programmers felt about my use of the cmd module and the efficiency of my functions.

Though this program in its current state is fully functional, I feel the design may be gridlocked and lacking flexibility, due to my lack of knowledge regarding the process of creating commands for the user to utilize.

The current design makes it difficult to implement the varying command attributes I hoped to add later on. How should I go about doing this? A format I see in other cmd programs goes something like command --numbers --property=number --filename

import cmd
def sym_extract(text,syms):
 text = text[text.find(syms[0])+1:text.find(syms[1])]
 return text
def del_punc(string):
 string = ''.join(ch if ch.isalnum() else '' if ch!=' ' else ' ' for ch in string)
 return(string.lower())
def cipher(text, shift):
 text = del_punc(text)
 secret = ''
 for c in text:
 if c in 'abcdefghijklmnopqrstuvwxyz':
 num = ord(c)
 num += shift
 if num > ord('z'):
 num -= 26
 elif num < ord('a'):
 num += 26
 secret = secret + chr(num)
 else:
 secret = secret + c
 return secret
class CryptoPromptCmd(cmd.Cmd):
 prompt = '\n> '
 def default(self, arg):
 print('invalid command, try typing "help"')
 def do_quit(self, arg):
 '''exits the program'''
 return True
 def do_encrypt(self, arg):
 '''usage: encrypt (provided string or /filename)[shift number]'''
 if arg != '':
 text = del_punc(sym_extract(arg,'()'))
 shift = int(sym_extract(arg,'[]'))
 encrypted = cipher(text,shift)
 print('\n ',encrypted.upper())
 else:
 print('invalid command\nusage: encrypt(provided string or /filename)[shift number]')
 return
 def do_decrypt(self, arg):
 '''usage:\noutput to file: decrypt(provided string or /filename)[file]\nattempt manually: decrypt(provided string or /filename)[shift number]'''
 if arg != '':
 text = del_punc(sym_extract(arg,'()'))
 shift = -1*int(sym_extract(arg,'[]'))
 decrypted = cipher(text,shift)
 print('\n ',decrypted.upper())
 else:
 print('invalid command\nusage:')
 print('output to file: decrypt(provided string or /filename)[output /filename]')
 print('attempt manually: decrypt(provided string or /filename)[shift number]')
 return
if __name__ == '__main__':
 print('\n'* 3)
 print("Caesar Cipher Tool")
 print('==================')
 print()
 print('type "help" for commands')
 print()
 CryptoPromptCmd().cmdloop()
 print('\nexiting...\n')
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jan 22, 2018 at 17:28
\$\endgroup\$
3
  • \$\begingroup\$ Did you consider something like tr d-za-c a-z? \$\endgroup\$ Commented Jan 22, 2018 at 18:38
  • \$\begingroup\$ I'm not really sure what you mean by that \$\endgroup\$ Commented Jan 23, 2018 at 0:45
  • \$\begingroup\$ On Unix the tr program translates characters according to the specification you give it, so tr a-z d-za-c encodes the Caesar cipher (with a shift of 3) and tr d-za-c a-z decodes it. For example, echo hello | tr a-z d-za-c outputs khoor. \$\endgroup\$ Commented Jan 23, 2018 at 12:46

2 Answers 2

3
\$\begingroup\$

If you're trying to make a shell-like utility to encrypt and decrypt strings the cmd module is exactly what you want.

In order to add argument lists to each command you could use the argparse module similarly to the following.

def do_decrypt(self, arg):
 parser = argparse.ArgumentParser(prog='decrypt')
 parser.add_argument('--filename', type=file, help='The file to decrypt')
 parser.add_argument('--shift', type=int, default=13, help='The amount to shift')
 try:
 args = parser.parse_args([arg])
 except SystemExit:
 print('The previous command was not understood')
 text = del_punc(args.file)
 shift = -1*args.shift
 decrypted = cypher(text, shift)
 ...

Some people may complain that the SystemExit exception shouldn't be caught you could also create a class that subclasses argparser.ArgumentParser and overloads the argparser.ArgumentParser.error function

answered Jan 22, 2018 at 17:54
\$\endgroup\$
0
1
\$\begingroup\$

I can't say much about cmd, yet the code could be simplified

  1. Instead of comparing to the lowercase alphabet hardcoded, you could do from string import ascii_lowercase
  2. Instead of the if else you could use a % modulus operand in your cipher function.
  3. Don't do empty returns but be explicit and return None
  4. Using argeparse for capturing cli arguments is indeed a great improvement as suggested by @Thomas Eldon Allred
  5. Your del_punc seems off, you could have used string.punctuation for that to make it more obvious. And do (c for c in text if c not in string.punctuation)

Edit

Cleanup of encryption methods

from string import ascii_lowercase, punctuation
START = ord('a')
def shift_letter(c, shift):
 return chr((ord(c) - START + shift) % 26 + START)
def ceasar(text, shift):
 return ''.join(shift_letter(ch, shift) if ch in ascii_lowercase
 else '' if ch in punctuation
 else ch
 for ch in text)
if __name__ == '__main__':
 s = "zzyx6666...."
 print(ceasar(s, 10))
answered Jan 22, 2018 at 22:10
\$\endgroup\$
1
  • \$\begingroup\$ When I first read this it made sense, but now I'm drawing a blank. How exactly would I go about converting the if else portion of the cipher function to a modulo operation? \$\endgroup\$ Commented Jan 24, 2018 at 13:59

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.