robotools / fontParts

The replacement for RoboFab
MIT License
137 stars 44 forks source link

Document how we handle dynamic properties #774

Open benkiel opened 1 week ago

benkiel commented 1 week ago

We have _get/set_base_something and _get/set_something for a reason (below). That reasoning should be in the documentation somewhere.

Reasoning from @typesupply:

The _get_base_something method calls the environment implementation _get_something. It then validates/normalizes the returned value before sending the value to the scripter.

Likewise, _set_base_something validates and normalizes the value from the scripter before passing it to the environment implementation _set_soemthing.

This makes sure that the scripter and the environment implementations don’t have to do any validation or normalization.

knutnergaard commented 1 week ago

I've tried to make the purpose of dynamicProperty objects a bit clearer than before in the updated class documentation, although the reason for the base/sub method structure isn't clarified. The reasoning doesn't seem much different from that of having both public and private equivalents, so I'm unsure whether such explicit documentation for dymic properties is necessary.

If it is, the class docstring of dynamicProperty seems like the most logical place for it. I can add it if so desired.

typesupply commented 1 week ago

That's fine with me. Thanks!

benkiel commented 1 week ago

Same — this is really for anyone wanting to implement fontParts into their app/objects/etc, to make it clear what the logic is.

knutnergaard commented 1 week ago

I see. On second thought, how well and where is the private/public method structure documented? Perhaps it makes sense to keep these parts of the documentation in the same place?

typesupply commented 1 week ago

It could go somewhere on here. It could say that the _base implemented in the base class does the normalization and all of that. It might be enough documentation for when we need to remember what all this does. 😆