Is it good Python style to write a function that has no effect other than potentially raise exceptions?

Question

Sometimes I find myself writing Python code that looks like this:

def check_stuff(param):
    if condition1(param):
        return "condition1" # These might be enum values, etc., instead of strings
    if condition2(param):
        return "condition2"

def func(param):
    cs = check_stuff(param)
    if "condition1" == cs:
        raise Exception1(param)
    if "condition2" == cs:
        raise Exception2(param)      
    #...

Here, the return value of check_stuff is only used to decide what exception to raise. Let's imagine that this is true of every time I call check_stuff, probably because I only call it once. Would it be better style to just raise the exceptions in check_stuff itself?

def check_stuff2(param):
    if condition1(param):
        raise Exception1(param)
    if condition2(param):
        raise Exception2(param)

def func2(param):
    check_stuff(param)
    #...

Answer 1

Yes, a function that just raises an exception is OK.

You can easily have functions like ensureWellFormed(email_address) or http_request.ensureUserLoggedIn() that raise well-documented exceptions. This de-clutters code, and many high-profile projects, e.g. Django, do use this style here and there.

If you use this approach, make sure your exception objects carry enough context data to understand and report what happened in an exception handler. 'User has no access to the URL requested' is nigh useless; 'User joerandom has no access to URL /foo/bar' is quite useful.

^{No, a parameterless function working with global state is a bad idea. Your example code shows functions that check global state. Referring to global state implicitly is usually a very bad idea. It creates hard-to-detect dependencies. If you happen to alter global state, especially implicitly, it creates weirdest, hard-to-detect bugs. Code that refers to global state is hard to test and hard to refactor, too. The global-state approach may appear easier now, but in a slightly longer term it will bite painfully. Note how in my examples above either a parameter is passed to a function for checking, or an object instance is used (which is also passed to a method as the self parameter).}

Answer 2

No, it would be worse

• functions should tell what they do. With a good naming, your check_stuff function indicates what it does. It should therefore be named e.g. checkConditionsValid depending on what it actually does. I suppose your function func also returns something or has some side effect in case that the conditions are. If not, that would be a design flaw, not the check_stuff function.

• Use dependency injection. Instead of heaving check_stuff rely on some global variable, pass the conditions to it as arguments. It not only makes your code easier to reason about it also makes it more readable.

Answer 3

Maybe

It can work to externalize validation logic as long as it is not externally visible and is used to enforce preconditions.

Say you have an object that operates on a database connection, and most of its functions require that the connection be set up and initialized. Rather than checking the connection and conditionally throwing an exception in each function, have one function that checks the connection which is called by each method that uses the connection before it is used. This realizes DRY and helps ensure consistency.

For this to work well, the validation method and the state it checks should be private to the object. This prevents other objects (including subclasses) from causing it to violate preconditions.