mentalisttraceur / python-macaddress

BSD Zero Clause License
19 stars 5 forks source link

`self.formats` vs `type(self).formats` #14

Closed mentalisttraceur closed 2 years ago

mentalisttraceur commented 2 years ago

Kinda like #6: the formats could be a class-only thing, or optionally an instance-specific thing. Should they be gotten from the instance instead of from the class?

I mostly reached a stable decision on this right after #6, matching that decision, but I wanted to open this issue to discuss/record the thinking for the format-specific stuff.

mentalisttraceur commented 2 years ago

First one observation: if both

  1. individual instances can have a formats attribute on the instance, and

  2. the repr uses self.formats, then

the representation of the instance can end up not matching any of the current formats on the class.

(Now obviously in principle you can always have a race condition where formats is mutated on the class to something incompatible with its previous value, after getting the representation of an instance using the previous formats value, but this is a deeper problem that can't easily be solved without actively fighting against Python, and so shouldn't be used to justify allowing or creating problems that can easily be solved or not created without fighting against how Python works).

mentalisttraceur commented 2 years ago

So verdict 1: __repr__ should definitely use type(self).formats, even if for example __str__ uses self.formats.

mentalisttraceur commented 2 years ago

If we have type(self).formats in one place but self.formats in another place, this almost necessarily creates documentation obligation.

Code which reasonably seems mistaken or non-obviously justified to someone with good attention to detail and high sensitivity to possible errors should probably actively explain why it is the way that it is.

If I see type(foo).bar in one place and foo.bar in another, I think either

depending on how much confidence and trust the rest of the code inspired.

Either way, this creates drag - as in friction, headwind. It is like a protruding hook you can snag on, and will snag on, if you are mentally vigilant for mistakes or omissions in both the code you read and yourself.

mentalisttraceur commented 2 years ago

So verdict 2: if both exist in the code, that code should also self-explain why both are in there, why it is intentional and correct, and what determines which to use in a given case.

(And there doesn't seem to be a good way to do this without a comment or comment-like docstring or other thing which can freely stay out-of-date as the code changes.)

mentalisttraceur commented 2 years ago

Furthermore, what does setting/mutating formats on an instance achieve?

Changing the output format. That's it. (Unless of course you overload __new__ or __init__, and then you can also change the input format on a per-instance basis without having to reimplement or copy any logic provided by HWAddress into your own methods).

But in any situation where you might do

my_address.formats = desired_formats

you could instead do

MySubclassWithDesiredFormats(my_address)

and in fact the second composes a little nicer because it is effectively a pure function call.

mentalisttraceur commented 2 years ago

Finally, in a sorta general universal pattern, I think mutable attributes on a class are often not great, and they tend to just empower usage which only makes understanding, verifying, and optimizing the code harder.

There are times when mutable fields on a thing are appropriate, but this ain't one of them, and I'd say mutable state is often uncritically overused.

So I don't want to be needlessly contributing to people learning the idea by example of implementing things with mutable state. (That's why early on in this library's development I decided to not mention setting formats on an instance as a valid way to control stringification formatting.)

mentalisttraceur commented 2 years ago

And that's why so far the verdict is to also switch formats lookups from the instance to the class in version 2.