Do you have a lot of short, single-use, private functions in your Python code?

I must admin I have usually just been using the first approach like you and left the itch unscratched. I like the nested function approach, and by using a decorator you could easily pull the functions out for individual testing.

testables = {}

def testable(func):
    testables[func.__name__] = func
    return func

def first():
    @testable
    def second():
        return 5
    return 5 + second()

print first()
print testables['second']()

Thoughts?

If the only issue with testing the inner functions is accessing them from outside (I.e., if you don't build them to capture variables from the containing function's scope), you can expose them on the function itself:

def outer():
    def inner():
        pass
    outer._inner = inner

def test_inner():
    outer._inner()  # and testing

Asbjørn, tikitu,

Solving the problem of the testability of the inner functions is interesting. tikitu, you're right, the inner functions would be restricted from accessing variables from the containing function (i.e. they could not be closures). In that case, we would not expect testability of the individual inner functions anyways, right?

Note that the inner functions are re-created each time the enclosing function is called. 'def' statements are just statements in Python; they create function objects and "bind" them in the current enclosing namespace. Inner functions can be used to make closures but they incur the overhead of creating a function object each time the 'def' statement runs, so I would avoid using them just for stylistic reasons.

I consider readability to be one of the main strengths of Python so if having a bunch of little one-off functions makes it easier to understand how the code works then they are well worth it in my opinion.

The same goes for "promoting" a function and its dedicated helper functions to a class. That is what namespaces are for, after all, and although it's somewhat unusual I see nothing wrong with using a class that way and have done so myself in the past.

Simon, thanks for the heads up about the overhead of using inner functions. Thanks for your input on using classes also. I want to write good code and like hearing how others handle these types of things.

You're very welcome. :)

That's the problem of not understanding how OOP works. I recommend reading Uncle Bobs 'Clean Code'. Only do one thing at a time. Same for classes. Only one behaviour. So far I have addressed 4 different behaviours.

1. IO (db access)
2. algorithm detection
3. hashing
4. and the password check itself

Throwing together functions into a class is not necessarily OOP. There are principles to follow. But normally a long way of experience/pain. Nowadays I prefer functional languages more. For me it is so much easier using functional languages for most of the tasks

Here is my OOP approach which from my POV not only is much cleaner and easier to read and also it's fully testable, extendable and replaceable, without changing the code from PasswordChecker, or the Hashfactory. You only add a new algorithm or replace it with another (maybe faster or fixed one) and put it into the hash_alogorithms map. This mapping/configuration can even be read from a config file. Thats how IoC and Dependency Injection works in the enterprise (Java, .NET). A little outdate, but I hope you enjoy this solution

class MyRepo(object):
    def get_password_hash(email_address):
        """Get the user's password hash from the database.
        """
        pass


BCRYPT = 'bcrypt'
SHA1 = 'sha1'


class MyBcrypt(object):
    """This is the NEW algorithm used for hashing existing passwords.
    """
    def __init__(self, target_hash):
        self._target_hash = target_hash
        self.hash_algorithm = BRCYPT

    def __call__(self, password):
        pass


class MySha1(object):
    """This is the OLD password hash algorithm.
    """
    def __init__(self, target_hash):
        self._target_hash = target_hash
        self.hash_algorythm = SHA1

    def __call__(self, password):
        pass


class HashFactory(object):
    def __init__(self, hasher_map):
        self._hashers = hasher_map

    def determine(self, hashed_text):
        """Determine if the hash algorithm is bcrypt or sha1
        """
        pass

    def create(self, hashed_text):
        alogrithm = self.determine(hashed_text)
        constructor = self._hashers[algorithm]
        hasher = constructor(hashed_text) 
        return hasher

    def is_valid(self, hasher):
        return hasher.algorithm != BCRYPT


class PasswordChecker(object):
    """Check if a user's supplied password is correct.
    """
    def __init__(self, repository, hash_factory):
        self._repo = repository
        self._hash_factory = hash_factory

    def __call__(self, email_address, password):
        db_password_hash = self._repo.get_password_hash(email_address)
        hasher = self._hash_factory.determine(db_password_hash)
        input_password_hash = hasher(password)
        password_correct = (input_password_hash == db_password_hash)
        if password_correct and self._hash_factory.is_obsolete(hasher):
            self._repo.change_password(email_address, password)
            return True
        return False


def check_password_example():
    repo = MyRepo()
    hash_algorithms =  {BCRYPT: MyBcrypt,
                        SHA1: MySha1}
    hash_factory = HashFactory(hash_algorithms)
    pw_check = PasswordChecker()
    pw_check("[email protected]","12345safepw")