Simple hashing code with excessive conditional statements

Question

Any thoughts on my code? It's a simple hashing code that runs from the command line.

There just appears to be a lot of condition if / elif entries in both of the main functions (hashingmethod and hashcalculator).

# hashcalculator.py
import sys
import hashlib
args = len(sys.argv)
def inputVerification():
 if sys.argv[1] == "--help":
 scriptHelp()
 elif args < 5:
 print "You have failed to execute the command correctly"
 print "Please type 'hashcalculator --help' for more information"
 else:
 filename = sys.argv[2]
 hashingMethod(filename)
def scriptHelp():
 print "\n"
 print "----------------------------------------"
 print "\tWelcome to Hash Calculator"
 print "----------------------------------------"
 print "\n"
 print "--help"
 print "\n"
 print "In order to calculate the hash value of a file, your command line syntax"
 print "must be written as below:-"
 print "\n"
 print "\thashcalculator -f [file] -h [hash method]"
 print "\teg. hashcalculator -f notepad.exe -h SHA1"
 print "\n"
 print "The recognised hashing methods are MD5, SHA1, SHA224, SHA256, SHA384 and SHA512" 
def hashingMethod(filename):
 hashmethod = sys.argv[4].upper()
 if hashmethod == "MD5":
 hashCalculator(filename, "md5")
 elif hashmethod == "SHA1":
 hashCalculator(filename, "sha1")
 elif hashmethod == "SHA224":
 hashCalculator(filename, "sha224")
 elif hashmethod == "SHA256":
 hashCalculator(filename, "sha256")
 elif hashmethod == "SHA384":
 hashCalculator(filename, "sha384")
 elif hashmethod == "SHA512":
 hashCalculator(filename, "sha512")
 else:
 print "You have not entered a valid hashing method"
 print "Please review the help document"
 scriptHelp()
def hashCalculator(filename, hashmethod):
 with open(filename, 'rb') as f:
 if hashmethod == "md5":
 m = hashlib.md5()
 elif hashmethod == "sha1":
 m = hashlib.sha1()
 elif hashmethod == "sha224":
 m = hashlib.sha224()
 elif hashmethod == "sha256":
 m = hashlib.sha256()
 elif hashmethod == "sha384":
 m = hashlib.sha384()
 elif hashmethod == "sha512":
 m = hashlib.sha512()
 else:
 scriptHelp()
 while True:
 data = f.read(8192)
 if not data:
 break
 m.update(data)
 print "\n\n"
 print "The %s hash of file %s is:" % (hashmethod.upper(), filename)
 print "\n", m.hexdigest()
if __name__ == "__main__":
 inputVerification()

Answer 1

You should really take a look at argparse: the argparse module makes it easy to write user-friendly command-line interfaces. It will even take care of writing the help for you.

Even if you will apply very little of the following (since you should really use the argparse module), I'll give you some advice anyway.

str.lower()

This:

if hashmethod == "MD5":
 hashCalculator(filename, "md5")
elif hashmethod == "SHA1":
 hashCalculator(filename, "sha1")
elif hashmethod == "SHA224":
 hashCalculator(filename, "sha224")
elif hashmethod == "SHA256":
 hashCalculator(filename, "sha256")
elif hashmethod == "SHA384":
 hashCalculator(filename, "sha384")
elif hashmethod == "SHA512":
 hashCalculator(filename, "sha512")

could just be:

hashCalculator(filename, hashmethod.lower())

Coding style

Follow the official coding style, the main rules are summarized here.

getattr()

This:

if hashmethod == "md5":
 m = hashlib.md5()
elif hashmethod == "sha1":
 m = hashlib.sha1()
elif hashmethod == "sha224":
 m = hashlib.sha224()
elif hashmethod == "sha256":
 m = hashlib.sha256()
elif hashmethod == "sha384":
 m = hashlib.sha384()
elif hashmethod == "sha512":
 m = hashlib.sha512()

could just be:

m = getattr(hashlib, hashmethod)

Multiple line strings

Don't do this:

print "\n"
print "----------------------------------------"
print "\tWelcome to Hash Calculator"
print "----------------------------------------"
print "\n"
print "--help"
print "\n"
print "In order to calculate the hash value of a file, your command line syntax"
print "must be written as below:-"
print "\n"
print "\thashcalculator -f [file] -h [hash method]"
print "\teg. hashcalculator -f notepad.exe -h SHA1"
print "\n"
print "The recognised hashing methods are MD5, SHA1, SHA224, SHA256, SHA384 and SHA512"

Do this instead:

msg = """
----------------------------------------
\tWelcome to Hash Calculator
----------------------------------------
--help
In order to calculate the hash value of a file, your command line syntax
must be written as below:-
\thashcalculator -f [file] -h [hash method]
\teg. hashcalculator -f notepad.exe -h SHA1
The recognised hashing methods are MD5, SHA1, SHA224, SHA256, SHA384 and SHA512"""
print msg

• thefragileomen: Thanks for the advice Rik. Some really good pointers made and some really obvious oversights on my behalf!!

Answer 2

import sys
import hashlib
args = len(sys.argv)

You don't really get anything by doing this. You only use it once. Just use len(sys.argv) instead of args.

def inputVerification():

Python convention is to have lowercase_with_underscores

 if sys.argv[1] == "--help":
 scriptHelp()

What if the user passed no arguments? This will fail.

 elif args < 5:
 print "You have failed to execute the command correctly"
 print "Please type 'hashcalculator --help' for more information"
 else:
 filename = sys.argv[2]
 hashingMethod(filename)

Don't call the next function at the end of the function. You should return the filename from this function and then call hashingMethod in the caller.

def scriptHelp():
 print "\n"
 print "----------------------------------------"
 print "\tWelcome to Hash Calculator"
 print "----------------------------------------"
 print "\n"
 print "--help"
 print "\n"
 print "In order to calculate the hash value of a file, your command line syntax"
 print "must be written as below:-"
 print "\n"
 print "\thashcalculator -f [file] -h [hash method]"
 print "\teg. hashcalculator -f notepad.exe -h SHA1"
 print "\n"
 print "The recognised hashing methods are MD5, SHA1, SHA224, SHA256, SHA384 and SHA512" 

You should this in a multiline string rather then lots of print statements.

def hashingMethod(filename):
 hashmethod = sys.argv[4].upper()
 if hashmethod == "MD5":
 hashCalculator(filename, "md5")
 elif hashmethod == "SHA1":
 hashCalculator(filename, "sha1")
 elif hashmethod == "SHA224":
 hashCalculator(filename, "sha224")
 elif hashmethod == "SHA256":
 hashCalculator(filename, "sha256")
 elif hashmethod == "SHA384":
 hashCalculator(filename, "sha384")
 elif hashmethod == "SHA512":
 hashCalculator(filename, "sha512")
 else:
 print "You have not entered a valid hashing method"
 print "Please review the help document"
 scriptHelp()

Instead of chains of IFs, put your supported methods in a list.

def hashCalculator(filename, hashmethod):
 with open(filename, 'rb') as f:
 if hashmethod == "md5":
 m = hashlib.md5()
 elif hashmethod == "sha1":
 m = hashlib.sha1()
 elif hashmethod == "sha224":
 m = hashlib.sha224()
 elif hashmethod == "sha256":
 m = hashlib.sha256()
 elif hashmethod == "sha384":
 m = hashlib.sha384()
 elif hashmethod == "sha512":
 m = hashlib.sha512()
 else:
 scriptHelp()

Instead of doing this, I'd suggest using a dictionary

 while True:
 data = f.read(8192)
 if not data:
 break
 m.update(data)
 print "\n\n"
 print "The %s hash of file %s is:" % (hashmethod.upper(), filename)
 print "\n", m.hexdigest()

Don't mix your output with your logic.

if __name__ == "__main__":
 inputVerification()

You only seem to read a few random entries from sys.argv. You don't look at argv[3] at all. You only pay attention if argv[1] is "--help".

Here's me reworking of your code:

import sys
import hashlib
HASH_METHODS = {
 'md5' : hashlib.md5,
 'sha1' : hashlib.sha1,
 'sha224' : hashlib.sha224,
 'sha256' : hashlib.sha256,
 'sha384' : hashlib.sha384,
 'sha512' : hashlib.sha512
}
USAGE = """
----------------------------------------
\tWelcome to Hash Calculator
----------------------------------------
--help
In order to calculate the hash value of a file, your command line syntax
must be written as below:-
hashcalculator -f [file] -h [hash method]
\teg. hashcalculator -f notepad.exe -h SHA1
The recognised hashing methods are MD5, SHA1, SHA224, SHA256, SHA384 and SHA512
"""
def hash_file(method, filename):
 m = method()
 with open(filename) as f:
 while True:
 data = f.read(8192)
 if not data:
 break
 m.update(data)
 return m.hexdigest()
def show_script_help():
 print USAGE
def main(args):
 if args[1:] == ['--help']:
 show_script_help()
 elif len(args) != 3:
 print "You have failed to execute the command correctly"
 print "Please type 'hashcalculator --help' for more information"
 else:
 filename, hash_method = sys.argv[1:]
 try:
 method = HASH_METHODS[hash_method]
 except KeyError:
 print "You have not entered a valid hashing method"
 print "Please review the help document"
 else:
 hashed = hash_file(method, filename)
 print "\n\n"
 print "The %s hash of file %s is:" % (hash_method.upper(), filename)
 print "\n", hashed
if __name__ == "__main__":
 main()

• thefragileomen: Many thanks for the advice @Winston. Much appreciate and thanks for the time in assisting me and reviewing my code. Some really good points made.

Answer 3

I'm not a Python developer, so this might be completely missing the point. But I look at the code for hashlib.py. And there is a chunk of code in the middle that looks a lot like yours, so I wonder if you're reinventing the wheel.

Looking further at the documentation for hashlib and it looks like it has a constructor which takes the name and returns a hashing object.

So you should be able to replace half of your code with a simple

m = hashlib.new(hashmethod)

• thefragileomen: Many thanks for the comments @pdr. The reason behind me creating this code is to simply practice coding in Python. There are quite a few hash calculators out there written in Python but I was just looking to start from scratch to learn as I go along.