Skip to content

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

Closed
PgBiel opened this issue Jun 4, 2020 · 4 comments
Closed
Labels
refactor Refactor or redesign of existing code

Comments

@PgBiel
Copy link
Member

PgBiel commented Jun 4, 2020

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:

  1. The use of @property (which allows custom code to run when access is attempted or an assignment is done);
  2. Direct access to the attribute (i.e. just delete the get and set functions where they don't do anything at all besides just getting or setting).

As I exposed in #122 (comment), rather than having a set function for each attribute, we should (also) implement a general set 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

@PgBiel PgBiel added the refactor Refactor or redesign of existing code label Jun 4, 2020
@PgBiel PgBiel added this to the PyPI Release 1.0.0 milestone Jun 4, 2020
@leotrs
Copy link
Contributor

leotrs commented Jun 4, 2020

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

@PgBiel
Copy link
Member Author

PgBiel commented Jun 4, 2020

Can we have here a clear distinction/guidelines for when to use just an attribute and when to use a property?

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, should we do this after the dataclass/attr refactor? So as to not duplicate efforts...

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

@behackl behackl removed this from the Release v0.1 milestone Oct 1, 2020
@leotrs
Copy link
Contributor

leotrs commented Oct 8, 2020

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 leotrs closed this as completed Oct 8, 2020
@PgBiel
Copy link
Member Author

PgBiel commented Oct 14, 2020

@leotrs but are we going to implement it? If so, it should be left open, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor or redesign of existing code
Projects
None yet
Development

No branches or pull requests

3 participants