Messages in this thread | | | Date | Mon, 4 Dec 2017 21:05:25 +0100 | From | Simon Sandström <> | Subject | Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h |
| |
On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: > > Hi Simon, hi Dan, > > if you both are of the same opinion, for me, it's fine, if we go with two > functions. > > But I don't get the advantage, if we split approx. 10 functions, to get rid > of enum optionOnOff. > > Keep in mind, that if you split the functions, in the interface > implementation you also need more code: > > SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); > > will have to be converted in something like > > if (rx_cfg->enable_sync) > SET_CHECKED(rf69_set_sync_enbable(dev->spi); > else > SET_CHECKED(rf69_set_sync_disable(dev->spi);
I think that this makes the code very clear. If the config tells us to enable the sync then we'll enabled it, otherwise we'll disable it.
> > For me, it is important, that the configuration, you'll have to write in the > user space program (aka fill out the config struct) will be 100% > non-ambigious and easy to read. > > Cheers, > Marcus
- Simon
| |