5

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) #...
Improve this question

3 Answers 3

Reset to default
8

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

Improve this answer
2
  • 1
    A good example: The requests library lets you raise exceptions from bad requests with Response.raise_for_status(). Commented May 16, 2018 at 17:08
  • Feel free to use that example in your answer. In particular, I'm fond of naming functions like raise_for_x() or raise_if_y(). Commented May 16, 2018 at 17:12
1

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.

Improve this answer
6
  • Well, foo also doesn't say what it does, but it's still a function name in plenty of examples. The global vars are bad practice, but I'm actually not sure that's what he was asking about. Commented Mar 2, 2015 at 16:26
  • foo does say what it does. It does what would be suitable in your real world code. It is therefore not intended to be used in real world code. ;) Using global vars influences how functions are named. You are right, it is not specifically what he asked for, but it may help understanding the better naming and the reason for that naming. Commented Mar 2, 2015 at 16:31
  • Your entire point is assuming he's posting real-world code. You're likely wrong, and he didn't ask to understand naming conventions. He asked about exception conventions. Commented Mar 2, 2015 at 16:34
  • The apparent use of global variables (which I did to keep the examples short) was so heinous that several people commented on it, so I removed it. Commented Mar 2, 2015 at 16:35
  • @Katana314 Those two things belong together. A function should be named different when throwing an exception or returning a value or executing a side effect. In my opinion these information should be included in the answer. Commented Mar 2, 2015 at 16:40
1

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.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.