3

I'm working on a little side project for work and have made some classes and methods. One of the classes represents a shelf in our inventory and another represents each a bin on the shelf. I have a method to add a new bin to the shelf and I added some logic to make sure it was passed a Location object before it is added to the shelf (right now I'm using lists while I develop before moving everything to a DB).

However I just read in a Python book that I have that it is usually better to handle exceptions as they arise rather than add extra code. I removed the logic to see what error I would get but I didn't get anything it allowed a string in place of the Location object.

Is there a more Pythonic way to enforce what type a parameter is?

What I have for the shelf:

class CrossDock:
locations = []

def add_location(self, location):
    if isinstance(location, Location): #if this is commented out it will take what ever is passed to it
        self.locations.append(location)
    else:
        print("location parameter must be of type: Location. Parameter is of type" + str(type(location)))

Is there a way I can do this with a try/except block?

Ray
  • 2,472
  • 18
  • 22
kylieCatt
  • 10,672
  • 5
  • 43
  • 51
  • Note: Your `locations` field is a class field, not an instance field. All `CrossDock` instances will share the same copy. To fix this, define an `__init__` method and set `self.locations` in there. – user2357112 Dec 19 '13 at 03:57
  • There will only be one instance of CrossDock, should I still put the locations field in an __init__ method? – kylieCatt Dec 19 '13 at 04:06
  • Yes. It conceptually belongs to that instance; if you were to make more `CrossDock`s, you would want them to have separate lists. – user2357112 Dec 19 '13 at 04:07

4 Answers4

4

Propagate an exception to the caller. This forces users of your class to fix the invalid type when they misuse the class. Printing is not useful as it does not enforce an interface contract.

class CrossDock(object):
    def __init__(self):
        self.locations = []

    def add_location(self, location):
        if isinstance(location, Location):
            self.locations.append(location)
        else:
            raise TypeError("location must be Location, got: " +
                            repr(type(location)))
jfs
  • 399,953
  • 195
  • 994
  • 1,670
mattypiper
  • 1,222
  • 8
  • 8
  • I think you have some Java leaking in, and you're missing parentheses on `__init__`. Also, not a bug, but I'd recommend having `CrossDock` inherit from `object` so it's a new-style class. – user2357112 Dec 19 '13 at 05:44
  • I've made it valid Python per @user2357112's comment and changed it to raise more idiomatic `TypeError`. Feel free to rollback – jfs Dec 19 '13 at 06:33
1

The exception you might eventually get would happen somewhere else in your code, when you tried to use a Location instance but found something else there instead. There's nothing wrong with checking a parameter when it comes from an unreliable source. This places the exception at the root of the problem, rather than at a potentially more difficult to diagnose secondary location in your code.

You might do pretty much what you have, only raise an exception rather than print the error.

def add_location(self, location):
    if not isinstance(location, Location):
        tmpl = "location parameter must be of type: Location, got %s"
        raise TypeError(tmpl % str(type(location)))
    ... do other processing here after guarding check ...

This type of checking is most appropriate if you have no control over the calling code. If you just want to catch a programming error you've made yourself, you can just use an assert:

def add_location(self, location):
    assert isinstance(location, Location), "location must be of type: Location"
    ... do other processing here

The advice against doing parameter type checks is directed at allowing the most flexibility in your code, like if someone wanted to pass in an object that had the same methods as location. Checking for a hard-coded type would raise an exception even though the code could work.

scanny
  • 26,423
  • 5
  • 54
  • 80
1

The most pythonic way is to not type check... everything is duck typed.

What's the canonical way to check for type in python?

Community
  • 1
  • 1
mattypiper
  • 1,222
  • 8
  • 8
-2

Use assert inside the try/except block

class Location():
    pass

class CrossDock:
    locations = []

    def add_location(self, location):
        try:
            assert(isinstance(location, Location))
            self.locations.append(location)
            print("added")
        except AssertionError:
            print("error: Parameter is of type" + str(type(location)))


c = CrossDock()
loc = Location()
c.add_location("2")
c.add_location(loc)

Will fail on the first add_location call

location parameter must be of type: Location. Parameter is of type<type 'str'>

added
Holy Mackerel
  • 3,259
  • 1
  • 25
  • 41
  • Asserts are strictly for debugging; they should not be used to validate preconditions of methods that are part of public interfaces, as they can be disabled. Also, catching the exception and printing an error message is much less helpful than just letting it propagate. – user2357112 Dec 19 '13 at 04:00
  • The OP asked for a try/except so that is why I caught the exception. The printing is for demonstration and is in line with the actions in the OP's question. – Holy Mackerel Dec 19 '13 at 04:05
  • What I have works but I'm not sure if it's the best way after doing some reading. SO I have something that works but I'd like to know if it's/what is the best way going forward. – kylieCatt Dec 19 '13 at 04:09