###Alternative implementation
Alternative implementation
###A few more general review points
A few more general review points
###Alternative implementation
###A few more general review points
Alternative implementation
A few more general review points
To address your questions:
- Are my assumptions correct?
If you mean the assumption that "public attributes are meant to be played around", then not necessarily. It's pretty well impossible to make things truly private in Python; just being public doesn't really mean "do whatever you like", which brings me to:
- Is it acceptable to assume that every developer will read the documentation of the classes I write
No, but it is acceptable to assume that if they don't read the documentation, misuse your code and end up in trouble it's their fault and problem! You will often hear the expression "we're all consenting adults here" in Python - the language has lots of dynamic and introspective features, so we pretty much have to operate on the basis that nobody is particularly trying to do the wrong thing.
- ...what if I cannot always return read-only versions of the attributes, or if it costs too much resources?
Then that's a trade-off you have to make and document. These issues come up a lot in software development; make the best decision you can at the time, write down why you made it and revisit it if a problem comes up later.
###Alternative implementation
All that being said, you're right that if the user can change e.g. center
, they will expect the points
to be updated accordingly. A third way would be to use caching, trading off storage space against speed. Consider:
class PointsAlongCircle(object):
_cache = {} # we will store the sequences of Point objects here
def __init__(self, center, radius, num_points):
self.center = Point(*center) # simpler way to deal with things!
self.radius = radius
self.num_points = num_points
def points(self):
"""Get the points on the circle."""
key = (self.center, self.radius, self.num_points)
if key not in self._cache:
self._cache[key] = self._calculate_points()
return self._cache[key]
def _calculate_points(self):
"""The actual calculations happen here, if the result isn't cached."""
...
For each combination of center
, radius
and num_points
, the tuple of points is calculated only once.
###A few more general review points
This seemed odd:
if isinstance(numpoints, int): # if the input is an integer
self._numpoints = int(numpoints) # convert it to an integer
else: # otherwise
raise ValueError('numpoints must be an integer') # complain about the 'value'?
As an alternative, if you do want to do duck-type checking:
try:
self._numpoints = int(numpoints)
except ValueError:
raise TypeError("can't convert {!r} to integer".format(numpoints))
However, note that this will convert float
arguments, when the correct behaviour might be to raise an error. Another alternative is to wait for the error to come from range
, although if you're using the above method that might be too late!
return tuple(self._points)
This also seemed odd - why not just make self._points
a tuple
to start with? This will be more efficient, as the current method creates a new object every time the property is called.