Skip to content

Get rid of "Get or set" methods #2

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
tannewt opened this issue Dec 7, 2017 · 12 comments
Closed

Get rid of "Get or set" methods #2

tannewt opened this issue Dec 7, 2017 · 12 comments

Comments

@tannewt
Copy link
Member

tannewt commented Dec 7, 2017

Get or set methods are a weird programming pattern inherited from MicroPython. It should be replaced with properties instead.

@caternuson
Copy link
Contributor

What's a good approach for using properties with 2D matrix like things? I think various syntaxes could be accommodated.

pixels[x][y] = color

or

pixels[x,y] = color

@deshipu
Copy link

deshipu commented May 16, 2018

Both have their problems. In the first case, you have to create an intermediate object with its own getters and setters. In the second case, you have to create the tuple. That makes them slow and gives one more opportunity for memory fragmentation.

@deshipu
Copy link

deshipu commented May 16, 2018

Also, it should be pixels[y][x], as it's arranged by rows first and columns second.

@tannewt
Copy link
Member Author

tannewt commented May 16, 2018

The Python pillow library does [x,y]: https://pillow.readthedocs.io/en/5.1.x/reference/PixelAccess.html

@caternuson
Copy link
Contributor

Works for me. Makes sense to follow suit with pillow. Any objections to going ahead and using this syntax for 2D property access?

@deshipu
Copy link

deshipu commented May 19, 2018

I wonder if you would also want to have slices there for doing filled rectangles?

@deshipu
Copy link

deshipu commented May 24, 2018

By the way, I would really recommend leaving the pixel method there, and only get rid of the blinking and brightness setters. There are many libraries that will work with anything that has a pixel method. Then again, you can probably just pass a framebuf object to them directly.

@caternuson
Copy link
Contributor

#11 did some of this for the HT16K33 class and the matrix like displays. But still need to do something for the segmented displays.

Could do something like this, which would just be a simple refactoring of the current code:

disp.text = "hello"
disp.number = 42

or maybe try and add something that acts more like print():

disp.print("hello")
disp.print(42)

@deshipu
Copy link

deshipu commented Jun 18, 2018

I like the latter option much more.

@tannewt
Copy link
Member Author

tannewt commented Jun 18, 2018 via email

@caternuson
Copy link
Contributor

caternuson commented Jun 21, 2018

I think with #12 , this should be done. OK to close?

@tannewt
Copy link
Member Author

tannewt commented Jun 25, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants