Is self.__dict__.update(**kwargs) good or poor style?
In Python, say I have some class, Circle, that inherits from Shape. Shape needs x- and y-coordinates, and, in addition, Circle needs a radius. I want to be able to initialize Circle by doing something like,
c = Circle(x=1., y=5., r=3.)
Circle inherits from shape, so I need to use named arguments to __init__
, because different classes require different constructors. I could manually set x, y, and r.
class Shape(object):
def __init__(self, **kwargs):
self.x = kwargs['x']
self.y = kwargs['y']
class Circle(Shape):
def __init__(self, **kwargs):
super(Circle, self).__init__(**kwargs)
self.r = kwargs['r']
or, I could have the attributes of my Circle set automatically using self.__dict__.update(kwargs)
class Shape(object):
def __init__(self, **kwargs):
self.__dict__.update(**kwargs)
class Circle(Shape):
def __init__(self, **kwargs):
super(Circle, self).__init__(**kwargs)
The advantage of this is that there's less code and I don't need to maintain boilerplate like self.foo = kwargs['foo']
. The disadvantage is that it isn't obvious which arguments are needed for Circle. Is this considered a cheat or is this good style (as long as the interface to Circle is well-documented)?
Thanks, everyone, for your thoughtful responses. The self.__dict__.update(**kwargs)
hack has been useful for me in experimenting with organizing my code, but I'll make sure that I replace that with properly passing arguments explicitly and doing clear error checking in production code.
Solution 1:
class Shape(object):
def __init__(self, x=None, y=None):
self.x = x
self.y = y
class Circle(Shape):
def __init__(self, r=None, **kwargs):
super(Circle, self).__init__(**kwargs)
self.r = r
And this is it. Don't use **kwargs
when you don't really need them.
Is this considered a cheat or is this good style (as long as the interface to Circle is well-documented)?
When you have a choice between writing a simple, understandable code and headache code + nice docstrings, you actually don't have any choices, you just go and write simple, self-documented code:)
Solution 2:
I would say that the first method is definitely preferable, because explicit is better than implicit.
Consider what would happen if you made a typo when initializing a Circle, something like Circle(x=1., y=5., rr=3.)
. You want to see this error immediately, which would not happen with __dict__.update(kwargs)
.
Solution 3:
If you wish to assign automatically, I suggest the following approach:
def __init__(self, **kwargs):
for key, value in kwargs.iteritems():
setattr(self, key, value)
which, in terms of style, is somewhere in between writing it explicitly and hacking it yourself using self.__dict__
.
Solution 4:
If you wanted it to be more obvious you could have Circle.__init__
perform some sanity checks on the arguments. Presumably you'd check to make sure all the arguments are there, and possibly raise errors for meaningless arguments.
You could probably even make a decorator or helper function in Shape
to do this for you. Something like this:
class Circle(Shape):
def __init__(self, **kwargs):
self.check(kwargs, 'x', 'y', 'r')
super(Circle, self).__init__(**kwargs)
.check
would be implemented in Shape
and essentially just verifies that all the arguments are in kwargs
, and possibly that no extra ones are (sorry, no code for that one - you can figure it out on your own). You could even have subclasses overload it to check for optional arguments, which you may want to handle differently than other arguments (i.e. give them a default value that wouldn't otherwise be assigned in Shape.__init__
.
Otherwise, if you document your interface, and it works the way it's documented, it's always alright. Anything else you do to make it work the way we "expect" it to (throwing exceptions for incorrect arguments) is a bonus.