44

In a Python class, what type of error should I raise from an instance method when some of the other attributes of the class must be changed before running that method?

I'm coming from a C# background where I would use InvalidOperationException, "the exception that is thrown when a method call is invalid for the object's current state", but I couldn't find an equivalent built-in exception in Python.

I've been raising ValueError ("raised when a built-in operation or function receives an argument that has the right type but an inappropriate value") when the problem is with the function parameters. I suppose this is technically an invalid value for the self parameter; is that the right way to treat it? For example, is this idiomatic: raise ValueError("self.foo must be set before running self.bar()")?

Justin
  • 6,611
  • 3
  • 36
  • 57
  • 1
    ValueError seems good to me. Its close enough for a user to identify its association with the problem. Its also not like python will slap you on the wrist for using the wrong exception. This is close enough to separate from other errors. – jdi May 23 '12 at 20:00
  • 1
    Why not make your own exception if you feel a need to give more detail? – Gareth Latty May 23 '12 at 20:00
  • 1
    @LattyWare: I have seen talks poo-pooing the willy nilly subclassing of new exception types. They suggest there are plenty of builtins and it only adds complexity. – jdi May 23 '12 at 20:03
  • @jdi the operative words being "if you feel a need to give more detail". Subclassing is pointless if nobody is going to catch it later, otherwise it's quite more useful than built-ins for example to tell apart user errors from syntax or internal errors. – MarioVilas Mar 28 '13 at 15:04

6 Answers6

40

ValueError is the best thing to raise in this case. For python, you should prefer using the built-in exception types over creating your own. You should only create new exception types when you expect that you will need to catch it and behave very differently than you'd behave when catching the builtin types. In this case, the situation shouldn't arise - you're not expecting to catch this because it would indicate an error in using the class in question. For this it's not worth creating a new type just to make it have another name - that's what the message string that you pass to ValueError() is for.

Is it possible to restructure your class so that such an invalid state is not possible?

Daenyth
  • 35,856
  • 13
  • 85
  • 124
  • 2
    Thanks -- I guess the important point is that no one would care to catch the exception (it would be a ["boneheaded exception"](http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx) in the client) – Justin May 24 '12 at 11:55
  • 1
    If nobody is going to catch it and it only serves as an internal check, you could use "assert" instead. When the expression in an assert is False, an AssertionError exception is raised. – MarioVilas Mar 28 '13 at 15:02
  • 6
    A good example for raising `ValueError` on a call in an invalid state can be found in Python itself: http://hg.python.org/cpython/file/v3.3.1/Lib/subprocess.py#l881 – Oben Sonne Feb 08 '14 at 18:18
  • 1
    @MarioVilas: asserts should never fail in production code, though, and indeed you can run a python interpreter with [assertions disabled](http://stackoverflow.com/questions/1273211/disable-assertions-in-python). You don't want to rely on asserts, unless no calling code should ever call the class in an invalid state... which you can't guarantee – Claudiu Apr 27 '16 at 15:08
  • 1
    I will just say it: it sucks that there's no separate InvalidStateError or similar, because it is an important difference, especially when debugging a stacktrace, if the issue is with a method parameter or with the usage of the object during a period of time. – Andrea Ratto Oct 19 '20 at 15:30
  • @ObenSonne is incorrect stating the referenced python is raised because of an invalid state. The exception there is because the *argument* passed is incompatible with the state. It's not such a good exception where there is no argument at all. I would really hesitate to have a `close()` method raise `ValueError('Already Closed!')`. There isn't an incorrect value there. – Philip Couling Aug 01 '22 at 08:53
  • @PhilipCouling, I see your point and agree. However, [here](https://github.com/python/cpython/blob/3cb9731b7e59b0df9a7a9ab6b7746a669958b693/Lib/graphlib.py#L75) is another CPython reference where `ValueError` is independent of parameters and only based on state. One could argue that `ValueError` still fits because on instance methods the object itself, given via `self`, is the invalid value (because it has an invalid state). That said, an explicit builtin`InvalidStateError` would be useful nevertheless I guess. – Oben Sonne Aug 28 '22 at 20:55
19

I find RuntimeError the most appropriate of all built-in exceptions to signal invalid state.

See this example of how it is used in CPython:

Python 2.7.10 (default, Jul 13 2015, 12:05:58)
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from threading import Thread
>>> Thread().join()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/threading.py", line 938, in join
    raise RuntimeError("cannot join thread before it is started")
RuntimeError: cannot join thread before it is started

It is important to notice that even the CPython implementation itself is not coherent about the use of specific exception types between libraries. Sometimes ValueError is used instead, however in my opinion its description from Python documentation shows that its use is reserved for other situations. RuntimeError is a more general exception and it should be used when a piece of code cannot behave correctly even if it was given proper input, which is somewhat similar to the situation when an object is in an invalid state.

Hermes
  • 756
  • 7
  • 10
2
class InvalidOperationException(Exception):
    pass

SYS_STATE = 1

def something_being_run():
    if SYS_STATE < 2:
        raise InvalidOperationException

Something like that you mean ? I see no reason why you shouldn't sub-class exception to make your own Exception types, but that might just be the old Oracle PL/SQL Dev in me coming out...

Christian Witts
  • 11,375
  • 1
  • 33
  • 46
  • 2
    If all you are doing is subclassing for a name and passing, its not neccessary. – jdi May 23 '12 at 20:14
  • 4
    It is necessary if someone is going to catch that exception later. It seems like a correct solution to me, although I agree OP was asking for a built-in exception instead. – MarioVilas Mar 28 '13 at 15:00
  • Rolling your own exception has a downside - nobody else is (nor should be) using it. – Tomasz Gandor Sep 13 '19 at 15:42
2

I think the pythonic way is not leave the object in such a state where a method call will not crash despite being in an erroneous state. These are the hardest bugs to find, as the point where the program finally topples over is no where the bug occurred.

eg.

class PseudoTuple(object):
    """
The sum method of PseudoTuple will raise an AttributeError if either x or y have
not been set
"""
    def setX(self, x):
        self.x = x

    def setY(self, y):
        self.y = y

    def sum(self):
        """
In the documentation it should be made clear that x and y need to have been set
for sum to work properly
"""
        return self.x + self.y

class AnotherPseudoTuple(PseudoTuple):
     """
For AnotherPseudoTuple sum will now raise a TypeError if x and y have not been 
properly set
"""
    def __init__(self, x=None, y=None):   
        self.x = x
        self.y = y

What should not be done is something like

class BadPseudoTuple(PseudoTuple):
    """
In BadPseudoTuple -1 is used to indicate an invalid state
"""
    def __init__(self, x=-1, y=-1):
        self.x = x
        self.y = y

    def sum(self):
        if self.x == -1 or self.y == -1:
            raise SomeException("BadPseudoTuple in invalid state")
        else:
            return self.x + self.y

I think this comes under the pythonic motto of:

It's easier to ask for forgiveness than it is to get permission

If the exceptional state is something that can be expected to happen during the normal course of execution rather than being a user error by misusing the class then it seems reasonable that you should create your own exception. StopIteration and iterators are an example of this.

Neuron
  • 5,141
  • 5
  • 38
  • 59
Dunes
  • 37,291
  • 7
  • 81
  • 97
  • 3
    I take the point that `None` is a better marker of invalid state than a special value like `-1`. In my case, the condition I am checking for is pretty complicated involving four different attributes, so I would like to include an error message to guide the user of the class, though. The purpose of the method is to send commands to an external system, so I can't just ignore invalid values. – Justin May 24 '12 at 12:06
  • 1
    Perhaps you might want to use `IOError` then. After all the error is to say there is something about the state of the object which means the communication cannot be carried out. – Dunes May 24 '12 at 12:26
-1

ValueError is okay to me, but I think AssertionError is more appropriate. Basically, it violates the assertion made by the API designer.

Jeremy Kao
  • 853
  • 10
  • 14
  • 6
    I think `AssertionError` is mostly reserved to `assert` and similar constructs, so you would probably just use `assert` statement to raise it. But that statement is sometimes ignored depending on interpreter configuration, because `assert` is usually intended for testing, not for production. So I think `AssertionError` is not the best choice for this state, as it will interfere with testing exceptions. – MarSoft Jun 26 '17 at 19:32
  • 1
    `AssertionError` is a poor choice for validation of external input, e.g. parameters passed to the library's API or whether the component invoked is in the right state for its method to be called. What `AssertionError` can be used for though in a production application build is validation of internal flows that the programmer has full control of. It is for situation that really should not happen, so the assertion helps during testing and it can be safely optimized-out during compilation for deployment on production environment. – Hermes Jul 25 '18 at 09:37
-2

I think you should raise a ValueError when the problem is with the function parameters, as you are doing, and an AttributeError when the problem is with an attribute that must be set.

Also, you could subclass the AttributeError to make a more specific Exception, but I don't see it necessary. The AttributeError Exception with your error message is clear enough.

Oskarbi
  • 297
  • 1
  • 8
  • 2
    AttributeError is when an attribute is missing, it has nothing to do with the internal logic state of your objects. – MarioVilas Mar 28 '13 at 15:01