Skip to content
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

Have a consistent getter/setter pattern #55

Open
kbeckmann opened this issue Apr 30, 2018 · 9 comments
Open

Have a consistent getter/setter pattern #55

kbeckmann opened this issue Apr 30, 2018 · 9 comments

Comments

@kbeckmann
Copy link
Member

I messed up here a bit, sometimes i use something_set, other times it's set_something.

What should we use? something_set feels a bit more api-ish than set_something, right? @arturo182

@arturo182
Copy link
Contributor

arturo182 commented Apr 30, 2018

I think it should be modulename_set_thing. I checked some of the drivers and it looks like only the power module is odd:

void power_dut_set_enabled(bool enabled);
err_t power_dut_get_enabled(bool *p_enabled);
err_t power_dut_ldo_set(uint32_t millivolt);
err_t power_dut_ldo_get(uint32_t *p_millivolt);

Should probably be:

void power_set_dut_enabled(bool enabled);
err_t power_get_dut_enabled(bool *p_enabled);
err_t power_set_dut_ldo(uint32_t millivolt);
err_t power_get_dut_ldo(uint32_t *p_millivolt);

@kbeckmann kbeckmann reopened this Apr 30, 2018
@kbeckmann
Copy link
Member Author

err_t power_set _dut_ldo
Why space 😪

@kbeckmann
Copy link
Member Author

kbeckmann commented Apr 30, 2018

After discussing with frazz and a few 🍻later, we think that power_dut_enabled_get makes the most sense.
Reason is that if you sort the calls it all makes sense. Module > narrower scope > set/get. Get and set should be close to each other.

@arturo182
Copy link
Contributor

I don't agree, it signs like Yoda speak, "power duty enabled get, hihihi" https://youtu.be/5I671B4MCC0

@kbeckmann
Copy link
Member Author

@EnJens we need your input 🤙

@EnJens
Copy link
Contributor

EnJens commented May 1, 2018

I think it should end in set/get as it's the "functionality" of the function rather than what it works on

@arturo182
Copy link
Contributor

So we should have

adc_callback_set
max14662_value_set
max14662_bit_set
max14662_value_cached_get
max14662_value_get
mcp4018t_value_set
mcp4018t_value_get
uart_handle_get

?

That all sounds so wrong :/

@kbeckmann
Copy link
Member Author

https://github.com/mossmann/hackrf/blob/master/firmware/common/max2837.h

They have some inconsistency there as well.. max2837_set_frequency vs max2837_reg_read/max2837_reg_write. Seems people mix these things up all the time.

@arturo182
Copy link
Contributor

I think this is the highest prio issue right now ;)

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