-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Replace getters and setters (where possible) by property or direct access #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Can we have here a clear distinction/guidelines for when to use just an attribute and when to use a property? Also, should we do this after the dataclass/attr refactor? So as to not duplicate efforts... |
In general, you will only use a property when the action of getting or setting requires extra code other than simply retrieving an attribute.
Also, yes, I'm just not sure about whether we should do it during the refactor or after, but perhaps we can do it after |
This issue was left open only because it depended on the implementing of attrs/dataclasses. But latest developments in #7 sound like that will never happen. Besides, I have already gotten rid of some get/set methods in some classes. |
@leotrs but are we going to implement it? If so, it should be left open, no? |
We should replace getters and setters, which are an antipattern in Python (as exposed on #122 (comment)). Most of them can be replaced either by:
@property
(which allows custom code to run when access is attempted or an assignment is done);As I exposed in #122 (comment), rather than having a
set
function for each attribute, we should (also) implement a generalset
function for dataclasses, which allows us to set multiple attributes at a time (and also to animate such setting), like so:mobj.set(x=5, y=6, z_index=50, ...)
Setter functions which require more than one parameter could be kept anyways, and then simply invoked by
.set
when required.Suggestions? Ideas?
Relevant PRs and comments (if there are more, please tell me or just edit this post): #122 (comment), #122 (comment), #116, #114
The text was updated successfully, but these errors were encountered: