Should the attributes of a class be part of the public interface even when they could break its methods?

Question

As a minimal example, consider the following (in Python):

class Foo:
    def __init__(self, bar):
        # Assume bar is a valid at this point
        self.bar = bar

    def divide_by_bar(self, num):
        return num / self.bar

If the attribute bar is public, my understanding is that it is valid for the client to do something like:

>>> foo = Foo(1)
>>> foo.bar = 0
>>> foo.divide_by_bar(2)
Traceback (most recent call last):
   ...
ZeroDivisionError: division by zero

We encountered a more complex version of this in our team and I suggested to make the attribute private, or read-only (or to consider the handling the exception). However, I got the reply that, since it is only used in one place in our code base, it is fine to leave it like that. I agreed to that, but I have noticed numerous examples of this practice in our code base and I am uncertain when should one be more strict with these cases. Should I ignore it until it is used in multiple places?

Answer 1

Python has no good language-level encapsulation mechanisms, so in a way this doesn't matter. But Python linters do recognize the “leading underscores mean private” convention, so respecting this encapsulation can be enforced.

Here, just adding an underscore will make the intention of the code much clearer. You shouldn't necessarily go and seek out any such code snippets to fix them, but if you do come across such an example, then improving it would be a no-brainer.

Alternatively, Python offers some mechanisms to make objects read-only. I like the attrs module for easily creating immutable classes, and with Python 3.7 you could also use dataclasses. Other approaches are probably not worth the effort, e.g. namedtuples have weird syntax and do stuff that you might not want (like making objects comparable).

Although the code could be improved (by clearly marking private fields as private, or by making the object immutable), this kind of Python code is not uncommon. If you want a safe programming language then Python is not for you. A lot of stuff in Python is merely a convention, and everyone should respect that convention. What is part of the public interface is not necessarily defined by a naming convention or by some real encapsulation mechanism, but often by what is mentioned in the documentation.

Answer 2

As a general rule, attributes of a class should be part of the public interface if and only if it makes sense within the contract of that class that callers would have access to that attribute.

If not all the values that can be assigned to the attribute are valid within the invariants that are supposed to hold for the class, then it might be better to not expose the attribute directly but only via a property (or an getter/setter pair of functions), so that any attempt to set an invalid value can be caught early.
The same holds if the outside world should only have read-only access to the attribute.

---

I got the reply that, since it is only used in one place in our code base, it is fine to leave it like that.

This I find a very dangerous attitude. This easily leads to arguments like "it was done like that before, so it must be acceptable to do it like that again" and before you know it you have to officially support the usages that were deemed inappropriate but innocent at first. That may severely hinder your possibilities to do maintenance later.

for that reason, you should push back on that one usage and even try to avoid the possibility that such one usage can creep in.

Answer 3

I am uncertain when should one be more strict with these cases

I would argue that one should be more strict when it's important to be more strict.

In your example you end up with a divide by zero problem. Is that bad? If this is an internal tool and the cause of the error is obvious because its only used by developers, then it's probably fine the way it is. If this is code for a nuclear reactor or medical equipment, maybe it's time to be a bit more strict.

You listed a couple of options, but you seem to have left out the option of creating a setter that could catch such an error. You can make bar public while still protecting against bad inputs.