4
\$\begingroup\$

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):

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 pts

The last line has a side effect: pts.center is updated, and it also breaks the consistency in pts.

Here is a second version of the class PointsAlongCircle which corrects these issues (version 2):

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
        '''
        # remark: convert the input in immutable types
        self._center = Point(x=float(center[0]), y=float(center[1]))
        self._radius = radius
        if isinstance(numpoints, int):
            self._numpoints = int(numpoints)
        else:
            raise ValueError('numpoints must be an integer')

        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]

    # Read-only attributes
    @property
    def center(self):
        return self._center

    @property
    def numpoints(self):
        return self._numpoints

    @property
    def radius(self):
        return self._radius

    @property
    def points(self):
        return tuple(self._points)  # return immutable to prevent any mess

I used properties to simulate read-only attributes, thus pts.center = something will raise an exception. And by converting the argument center in an immutable Point, I prevent any other modification of pts.center. Someone can still directly modify pts._center, but it is not my business anymore (consenting adults do not play with private attributes).

Same for pts.points: I return an immutable object to prevent things like this:

 points = pts.points
 points[0] = something

Here are now my questions:

  1. Are my assumptions correct?
  2. Version 2 of PointsAlongCircle seems more robust than version 1, but I am not convinced that it is better code. What do you think?
  3. Is it acceptable to assume that every developer will read the documentation of the classes I write, and never try to change public attribute unless it is explicitly written that it is permitted? Or is it my job to lock attribute which are supposed to be read-only, even if I feel a bit like a control freak?
  4. And what if I cannot always return read-only versions of the attributes, or if it costs too much resources?

A few last remarks:

  • I do not pretend that using a class in this case is a very good choice, a function which returns a named tuple does the job. This code is meant to illustrate several questions I have.
  • I also know that I can improve the code with magic methods (to have len(pts) instead of pts.numpoints). But this is not the main concern of this topic.
  • I could also use properties to compute the attributes center and numpoints from points (example: return len(self.points)), instead of saving them. But let's assume we cannot due this because computing the attributes takes too much resources.
\$\endgroup\$
  • \$\begingroup\$ Why do you need PointsAlongCircle to be a class? Instead, you could make it a class method of Point, so you'd use pts = Point.points_along_circle(center, 3, 4) instead of pts = PointsAlongCircle(center, 3, 4), and get a list instead of the custom object. Also, note that 'radium' != 'radius'! \$\endgroup\$ – jonrsharpe Jun 2 '15 at 12:08
  • \$\begingroup\$ @jonrsharpe Point.points_along_circle would have been better, you are right. I meant to illustrate complex objects, where it is important to save all the parameters and the output (which possibly takes time to compute), in order for example to save/load them to/from file. Thanks for radium != radius. \$\endgroup\$ – nbedou Jun 2 '15 at 12:32
3
\$\begingroup\$

To address your questions:

  1. 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:

  1. 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.

  1. ...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.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your explanation, I should more rely on the documentation to describe what is permited and what is not. I don't really need the alternative implementation in my case: I'd rather have center, numpoints or radius always constant in an instance. About the int thing, I wanted to raise an error if there is a float, and ensure I save an int and not other types which look like int, numpy.int for example. And yes, self._points should be a tuple. \$\endgroup\$ – nbedou Jun 3 '15 at 6:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.