patternpythonMinor
Ensuring data consistency in a PointsAlongCircle object
Viewed 0 times
pointsalongcircleconsistencyobjectdataensuring
Problem
I'd like to write classes which are Pythonic, readable and easy-usable. A main issue for me is to keep data consistency. Here is an example (version 1):
This code displays as expected
This code works. But in Python, public attributes are meant to be played around, right? So someone might legitimately try
Also in Python, there is duck-typing. Well, another developer might pass do something like this:
The last line has a side effect:
He
from collections import namedtuple
import math
Point = namedtuple('Point', ['x', 'y'])
class PointsAlongCircle(object):
'''Set of regularly spaced points along a circle'''
def __init__(self, center, radius, numpoints):
'''
center -- center of the circle (Point)
radius -- radius of the circle (float)
numpoints -- number of points along the circle
>>> pts = PointsAlongCircle(Point(0, 0), 3, 4)
>>> pts.points
'''
self.center = center
self.radius = radius
self.numpoints = numpoints
center_x, center_y = self.center
angles = [i * 2 * math.pi / numpoints for i in range(numpoints)]
self.points = [Point(center_x + radius*math.cos(angle), center_y + radius*math.sin(angle)) for angle in angles]
pts = PointsAlongCircle(center=Point(0, 0), radius=3, numpoints=4)
print(pts.points)This code displays as expected
[
Point(x=3.0, y=0.0),
Point(x=1.8369701987210297e-16, y=3.0),
Point(x=-3.0, y=3.6739403974420594e-16),
Point(x=-5.51091059616309e-16, y=-3.0)
]This code works. But in Python, public attributes are meant to be played around, right? So someone might legitimately try
pts.center = Point(1, 2) and expect that pts.points will be updated to be centered around this new point, which is in reality not the case. And data consistency in the object is broken: the actual center of pts.points is not anymore pts.center.Also in Python, there is duck-typing. Well, another developer might pass do something like this:
center = [0, 0]
pts = PointsAlongCircle(center=center, radius=3, numpoints=4) # so far so good
center[0] = 1. # breaks data consistency in ptsThe last line has a side effect:
pts.center is updated, and it also breaks the consistency in pts.He
Solution
To address your questions:
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:
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.
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.
For each combination of
A few more general review points
This seemed odd:
As an alternative, if you do want to do duck-type checking:
However, note that this will convert
This also seemed odd - why not just make
- 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.Code Snippets
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."""
...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'?try:
self._numpoints = int(numpoints)
except ValueError:
raise TypeError("can't convert {!r} to integer".format(numpoints))return tuple(self._points)Context
StackExchange Code Review Q#92458, answer score: 3
Revisions (0)
No revisions yet.