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

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()

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)