Reputation: 736
As far as I understand, the difference between @classmethod
and @staticmethod
is the cls
parameter which is used so a class can construct instances of subclasses. I have encountered some issues with that concept while trying to develop a library. An example:
class Line:
def __init__(self, length):
self.length = length
@classmethod
def unit(cls):
return cls(1)
def clone(self):
return Line(self.length)
def __mul__(self, other):
return Square(self.length * other.length)
class Square:
def __init__(self, area):
self.area = area
So the fact that the unit
factory is a classmethod
(as is common) and uses cls(...)
for object construction signals to me: "Feel free to inherit from this class, it was designed for that." Otherwise it could just use Line(1)
for construction. But now lets say a wild user appears and actually inherits from the classes, implementing their own functionality on top:
class ColoredLine(Line):
def __init__(self, length, color):
super().__init__(length)
self.color = color
class ColoredSquare(Square):
def __init__(self, area, color):
super().__init__(area)
self.color = color
Issue 1: As far as I know, it is perfectly legit to modify the constructor in order to add parameters when inheriting. However, Line
is not aware of this and if its unit
factory tries to call ColoredLine.__init__
without the new required color
parameter, it throws. So a user is actually required to maintain compatibility with the old constructor (which might be very complex and change as the library evolves). But ok, the user adds a default color="red"
and we move on.
Issue 2: We should not differentiate between factory class methods and instance methods that return antoher instance, like clone
, so that should actually call type(self)(self.length)
instead of Line(...)
. This would however make the clone of a blue line become a red (default-colored) line with the same length. So should the base clone
instead generically copy all fields using __dict__
?
Issue 3: There is no way for __mul__
to know that it should now return a ColoredSquare
. So the pythonic inheritance considerations inconsistently only work for methods that return an object of the same type, so __add__
should work in most cases and __mul__
should not, __sub__
might.
The question is, where should be the line between already semi implementing parts of the functionality of an inheriting class and what to leave to the responsibility of the user? Should a user be required to sift through the complete implementation of a class and decide method by method if it needs overriding?
The date
object from datetime
faces all these problems. It silently requires constructor compatibility with its classmethod
factories, some methods return type(self)(...)
, ignoring any additional instance members and __sub__
will always return a timedelta
.
In my library, I am considering to less pythonically but more consistently use @staticmethod
and MyClass()
for factories instead of @classmethod
, cls()
and type(self)()
. A user is free to subclass but everything is their responsibility. That also makes type annotations much easier.
Or are there clear guidelines on this (including recommendations on how to deal with these issues instead of just recommending to use @classmethod
for factories)? Are there conceptual mistakes in my reasoning?
Upvotes: 2
Views: 70
Reputation: 12130
I'm unaware of the best practice for this and I don't think you can limit the user (programmer) to mess things up. What you can do is look at the implementation of the subclass and warn the programmer about possible issues:
class Line:
def __init__(self, length):
self.length = length
def __init_subclass__(cls, **kwargs):
# Find out how many required arguments are in the overridden __init__ method
if cls.__init__.__code__.co_argcount - len(cls.__init__.__defaults__ or ()) > 2:
# Use logger and update the warning as needed
print(f'Warning: class {cls.__name__} must implement `__init__` with only one required positional argument '
f'or override `unit` and `clone` methods for them to work properly.')
@classmethod
def unit(cls):
return cls(1)
def clone(self):
return type(self)(self.length)
class LineWithWidth(Line):
def __init__(self, length, width=5):
super().__init__(length)
self.width = width
class ColoredLine(Line):
def __init__(self, length, color):
super().__init__(length)
self.color = color
and the output:
Warning: class ColoredLine must implement `__init__` with only one required positional argument or override `unit` and `clone` methods for them to work properly.
Note that LineWithWidth
doesn't produce the warning.
You can do a similar thing with Square
.
Upvotes: 0