69

In general, let's say you have a method like the below.

def intersect_two_lists(self, list1, list2):
    if not list1:
        self.trap_error("union_two_lists: list1 must not be empty.")
        return False
    if not list2:
        self.trap_error("union_two_lists: list2 must not be empty.")
        return False
    #http://bytes.com/topic/python/answers/19083-standard
    return filter(lambda x:x in list1,list2)

In this particular method when errors are found, I would not want to return the empty list in this case because that could have been the real answer to this specific method call, I want to return something to indicate the parameters were incorrect. So I returned False on error in this case, and a list otherwise (empty or not).

My question is, what is the best practice in areas like this, and not just for lists?Return whatever the heck I want and make sure I document it for a user to read? :-) What do most of you folks do:

  1. If on success you were supposed to return True or False and you catch an error?
  2. If on success you were supposed to return a list and you catch an error?
  3. If on success you were supposed to return a file handle and you catch an error?
  4. et cetera
Bhargav Rao
  • 50,140
  • 28
  • 121
  • 140
TallPaul
  • 1,001
  • 4
  • 10
  • 12
  • 1
    Well as I see it: The intersection with an empty list results just in an empty list and not an exception. – stefanw Oct 27 '09 at 13:21
  • 1
    Well, I DID say "in general". :-) In other words, I was trying to concentrate on how to handle the return value(s) or exceptions rather than the problem the method tries to solve. And I think the question has raised some good discussion I am learning from. – TallPaul Oct 27 '09 at 14:39

7 Answers7

76

First, whatever you do don't return a result and an error message. That's a really bad way to handle errors and will cause you endless headaches. If you need to indicate an error always raise an exception.

I usually tend to avoid raising errors unless it is necessary. In your example throwing an error is not really needed. Intersecting an empty list with a non empty one is not an error. The result is just empty list and that is correct. But let's say you want to handle other cases. For example if the method got a non-list type. In this case it is better to raise an exception. Exception are nothing to be afraid of.

My advice for you is to look at the Python library for similar functions and see how Python handles those special cases. For example have a look at the intersection method in set, it tends to be forgiving. Here I'm trying to intersect an empty set with an empty list:

>>> b = []
>>> a = set()
>>> a.intersection(b)
set([])

>>> b = [1, 2]
>>> a = set([1, 3])
>>> a.intersection(b)
set([1])

Errors are only thrown when needed:

>>> b = 1
>>> a.intersection(b)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'int' object is not iterable

Sure, there are cases where returning True or False on success or failure can be good. But it is very important to be consistent. The function should always return the same type or structure. It is very confusing to have a function that could return a list or a boolean. Or return the same type but the meaning of this value can be different in case of an error.

EDIT:

The OP says:

I want to return something to indicate the parameters were incorrect.

Nothing says there is an error better than an exception. If you want to indicate the parameters are incorrect then use exceptions and put a helpful error message. Returning a result in this case is just confusing. There might other cases where you want to indicate that nothing has happened but it is not an error. For example if you have a method that deletes entries from a table and the entry requested for deletion does not exist. In this case it might be fine to just return True or False on success or failure. It depends on the application and the intended behaviour

Nadia Alramli
  • 111,714
  • 37
  • 173
  • 152
  • 1
    @S.Lott, I said nothing that contradicts with this. But it is very important to look at the library for guidance to be consistent. There is nothing wrong with that and it should be encouraged. – Nadia Alramli Oct 27 '09 at 13:35
  • @S.Lott, I edited the answer to clear the exception point further. – Nadia Alramli Oct 27 '09 at 13:46
  • 2
    +1: returning a consistent result is definitely a requirement from the usability standpoint. – D.Shawley Oct 27 '09 at 13:50
  • I just looked at sets.py in Python 2.5.4. It seems to be returning an inconsistent return type in it's code in the __or__ method (and others). For example, if not isinstance(other, BaseSet): return NotImplemented return self.union(other) This will either return a constant (NotImplemented) or the union of the two sets. Am I reading this incorrectly? – TallPaul Oct 27 '09 at 15:10
  • 1
    @TallPaul, you are looking at __add__ not intersection and there is a different between the two. __add__ is overriding the & operator. NotImplemented is returned as a flag to Python: "For most intents and purposes, an operator that returns NotImplemented is treated the same as one that is not implemented at all". If the other operand doesn't support the operator as well Python raises an error. Try it. For more defailts: http://docs.python.org/reference/datamodel.html#object.__or__ – Nadia Alramli Oct 27 '09 at 15:27
  • @TallPaul, another good explanation: http://jcalderone.livejournal.com/32837.html – Nadia Alramli Oct 27 '09 at 15:35
  • Nadia, thanks for your time and your engery in all of your explanations to a Python rookie! – TallPaul Oct 28 '09 at 03:06
  • Agreed to checking how stdlibs deal with similar situations. – kakyo Jul 15 '19 at 01:54
25

It'd be better to raise an exception than return a special value. This is exactly what exceptions were designed for, to replace error codes with a more robust and structured error-handling mechanism.

class IntersectException(Exception):
    def __init__(self, msg):
        self.msg = msg
    def __str__(self):
        return self.msg

def intersect_two_lists(self, list1, list2):
    if not list1:
        raise IntersectException("list1 must not be empty.")
    if not list2:
        raise IntersectException("list2 must not be empty.")

    #http://bytes.com/topic/python/answers/19083-standard
    return filter(lambda x:x in list1,list2)

In this specific case though I'd probably just drop the tests. There's nothing wrong with intersecting empty lists, really. Also lambda is sort of discouraged these days in preference to list comprehensions. See Find intersection of two nested lists? for a couple of ways to write this without using lambda.

gdvalderrama
  • 713
  • 1
  • 17
  • 26
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
  • 2
    This doesn't really test against empty lists only though. Consider calls like `intersect_two_lists(0, 0)` or `intersect_two_lists('', None)`. They are both incorrect but the exception would state that the `list1` was empty when it wasn't even the correct type. – D.Shawley Oct 27 '09 at 13:45
  • 2
    You are right this doesn't check specifically for empty list, but neither did the original code. Python style is more to go ahead and use an object rather than fret beforehand whether it is a specific type. – Ned Batchelder Oct 27 '09 at 14:21
16

I like to return a tuple:

(True, some_result)

(False, some_useful_response)

The some_useful_response object can be used for handling the return condition or can serve to display debugging info.

NOTE: this technique applies for return values of any sort. It should not be mistaken with exception cases.

On the receiving end, you just have to unpack:

Code, Response = some_function(...)

This technique applies for the "normal" control flow: one must use the exception functionality when some unexpected inputs / processing occur.

Also worth noting: this technique helps normalizing function returns. Both the programmer and the user of the functions know what to expect.

DISCLAIMER: I come from an Erlang background :-)

Community
  • 1
  • 1
jldupont
  • 93,734
  • 56
  • 203
  • 318
  • +1: interesting thought. I don't know why it never occurs to me to return `(status, result)` tuples. – D.Shawley Oct 27 '09 at 13:46
  • 1
    +1: I was about to answer the same :-D Exceptions are, as their name indicates, for exceptional situations, for validating inputs that you except to be faulty on a normal basis (e.g. input from keyboard), it's usually better to return an error code... remember the Zen: explicit is better than implicit! :-) – fortran Oct 27 '09 at 14:28
  • 10
    I can't believe this is being encouraged. It is a design nightmare to use error codes. Check Ned's article for more details: http://nedbatchelder.com/text/exceptions-vs-status.html – Nadia Alramli Oct 27 '09 at 14:33
  • 1
    @nadia: I believe you are probably mistaking *return value* with *exception*. I am proposing a *return value* technique. – jldupont Oct 27 '09 at 15:06
  • 5
    @nadia: returning tuples is another method and quite a valid one. Many of the "exception vs. status" arguments are specific to languages that make it difficult to return multiple things and capture the result - i.e., where you can't do `status, result = f()` and you are forced to do something silly like `result = f(&status);` to achieve the same thing. Tuples are first class types in Python so we might as well use them. If this is done consistently throughout an API, it is quite elegant and efficient. – D.Shawley Oct 27 '09 at 17:22
  • 1
    @D.Shawley: The point behind exceptions is to not have to mix status codes with results; while returning `tuples()` are definitely a step above, the pythonic method is to use exceptions (which is several steps above ;). – Ethan Furman Nov 09 '11 at 18:10
  • This is the correct way. Not to raise exception. This way, one checks the status first. If the status says it is passed, then they can use the result, else not. Also, some systems return an error message in addition to status code. – Nasser Jun 19 '15 at 23:49
  • When going down this route I prefer [named tuples](https://pymotw.com/2/collections/namedtuple.html) for greater clarity: `from collections import namedtuple` `Outcome = namedtuple('Outcome', 'success msg')` – brokkr Jan 19 '17 at 08:29
  • Too many commenters forget that exceptions do not have referential transparency. I would always go for Either. – Wojciech Migda Aug 27 '18 at 13:49
11

Exceptions are definitely better (and more Pythonic) than status returns. For much more on this: Exceptions vs. status returns

Ned Batchelder
  • 364,293
  • 75
  • 561
  • 662
3

The general case is to throw exceptions for exceptional circumstances. I wish that I could remember the exact quote (or who said it), but you should strive for functions that accept as many values and types as is reasonable and maintain a very narrowly defined behavior. This is a variant of what Nadia was talking about. Consider the following usages of your function:

  1. intersect_two_lists(None, None)
  2. intersect_two_lists([], ())
  3. intersect_two_lists('12', '23')
  4. intersect_two_lists([1, 2], {1: 'one', 2: 'two'})
  5. intersect_two_lists(False, [1])
  6. intersect_two_lists(None, [1])

I would expect that (5) throws an exception since passing False is a type error. The rest of them, however, make some sort of sense but it really depends on the contract that the function states. If intersect_two_lists were defined as returning the intersection of two iterables, then everything other than (5) should work as long as you make None a valid representation of the empty set. The implementation would be something like:

def intersect_two_lists(seq1, seq2):
    if seq1 is None: seq1 = []
    if seq2 is None: seq2 = []
    if not isinstance(seq1, collections.Iterable):
        raise TypeError("seq1 is not Iterable")
    if not isinstance(seq2, collections.Iterable):
        raise TypeError("seq1 is not Iterable")
    return filter(...)

I usually write helper functions that enforce whatever the contract is and then call them to check all of the preconditions. Something like:

def require_iterable(name, arg):
    """Returns an iterable representation of arg or raises an exception."""
    if arg is not None:
        if not isinstance(arg, collections.Iterable):
            raise TypeError(name + " is not Iterable")
        return arg
    return []

def intersect_two_lists(seq1, seq2):
    list1 = require_iterable("seq1", seq1)
    list2 = require_iterable("seq2", seq2)
    return filter(...)

You can also extend this concept and pass in the "policy" as an optional argument. I would not advise doing this unless you want to embrace Policy Based Design. I did want to mention it just in case you haven't looked into this option before.

If the contract for intersect_two_lists is that it only accepts two non-empty list parameters, then be explicit and throw exceptions if the contract is breached:

def require_non_empty_list(name, var):
    if not isinstance(var, list):
        raise TypeError(name + " is not a list")
    if var == []:
        raise ValueError(name + " is empty")

def intersect_two_lists(list1, list2):
    require_non_empty_list('list1', list1)
    require_non_empty_list('list2', list2)
    return filter(...)

I think that the moral of the story is whatever you do, do it consistently and be explicit. Personally, I usually favor raising exceptions whenever a contract is breached or I am given a value that I really cannot use. If the values that I am given are reasonable, then I try to do something reasonable in return. You might also want to read the C++ FAQ Lite entry on exceptions. This particular entry gives you some more food for thought about exceptions.

Community
  • 1
  • 1
D.Shawley
  • 58,213
  • 10
  • 98
  • 113
  • Very nice, thanks. D, how do you document what all exceptions anyone calling your methods would have to handle? What I mean is, if you put in the __doc__ section of the method 'raises exceptions TypeError and ValueError (via calls to require_non_empty_list, etc)' and then later you change/add/edit/delete the exceptions require_non_empty_list raises... how do you ensure that all methods using require_non_empty_list have been properly re-documented? Sorry for the wordy comment! :-) – TallPaul Oct 27 '09 at 15:28
  • Generally, the caller should expect any exception and not specific types. The key is that exceptions should have common meaning regardless of their context. Making the meaning of an exception depend on where it is generated is a recipe for disaster. This is one of the primary reasons that I reserve exceptions for exceptional circumstances. If there is a problem that can be _dealt with_ in the function, then you shouldn't be raising an exception you should be defining how the function behaves in the identified edge case. – D.Shawley Oct 27 '09 at 17:26
  • ... oh... as for documentation. That is where you document the behavior in edge cases and the pre-conditions of the function. _"intersect_two_lists(Iterable,Iterable) -> list"_ states that you had better be sending two iterables in. If you don't `intersect_two_lists` is going to fail. – D.Shawley Oct 27 '09 at 17:28
1

For collections (lists, sets, dicts, etc.) returning an empty collection is the obvious choice because it allows your call site logic to remain clear of defensive logic. More explicitly, an empty collection is still a perfectly good answer from a function from which you expect a collection, you do not have to check that the result is of any other type, and can continue your business logic in a clean manner.

For non collection results there are several ways to handle conditional returns:

  1. As many answers have already explained, using Exceptions is one way to solve this, and is idiomatic python. However, it is my preference not to use Exceptions for control flow as I find it creates ambiguity on the intentions of an Exception. Instead, raise Exceptions in actual exceptional situations.
  2. Another solution is to return None instead of your expected result but this forces a user to add defensive checks everywhere in their call sites, obfuscating the actual business logic they are trying to actually execute.
  3. A third way is to use a collection type that is only capable of holding a single element (or being explicitly empty). This is called an Optional and is my preferred method because it allows you to keep clean call site logic. However, python does not have a built in optional type, so I use my own. I published it in a tiny library called optional.py if anyone wants to give it a try. You can install it using pip install optional.py. I welcome comments, feature requests, and contributions.
Chad Befus
  • 1,918
  • 1
  • 18
  • 18
0

There are plenty of arguments to make about why we should or should not use exceptions. Not to argue about that. But to answer the question directly, here is what we can do with Python 3.10+:

from typing import TypeVar, Generic

ResultType = TypeVar('ResultType')


# A generic success class that hold a value
class Success(Generic[ResultType]):
    def __init__(self, value: ResultType):
        self.value = value


# A 'Failure' type that hold an error message 
class Failure:
    def __init__(self, message: str):
        self.message = message


# A union type 'Result' that can either be 'Success' or 'Failure'
Result = Success[ResultType] | Failure

And here is an example of how it can be used:

# A function that will only return 'Success' when the greeting message start with "Hello" 
def make_a_call(greeting: str) -> Result:
    if greeting.startswith("Hello"):
        return Success(greeting)
    else:
        return Failure("Not polite")


if __name__ == '__main__':
    for sentence in ("Hello", "Hey"):
        response = make_a_call(sentence)
        if isinstance(response, Success):
            print("Greeting message:", response.value)
        else:
            print("Failure:", response.message)

Example Output:

Greeting message: Hello
Failure: Not polite
Yuchen
  • 30,852
  • 26
  • 164
  • 234