Tommie wrote:Hamburger wrote:Now that is less readable and looks - not aesthetically good.
But at least now it's plain obvious that these are boolean flags; something that wasn't the case with the old variable names.
Also, following our latest coding style agreements it still needs fixing and will end up as
Code: Select all
if (rcOptions[BOXGPSHOLD]) {
if (!GET_FLAG(FLAG_GPS_HOLD_MODE) & !GET_FLAG(FLAG_GPS_HOME_MODE)) {
SET_FLAG(FLAG_GPS_HOLD_MODE, 1);
I disagree. Just because the functions have turned into macros their name shouldn't need to change.
You are entitled to your opinion. But it still is against the coding style that was agreed on recently. In case you cannot find it
viewtopic.php?f=8&t=1412 And before you get on your high horse, it was not _my_ decision but consensus.
Tommie wrote:Hamburger wrote:Is there any benefit in this that could justify such eye sore (with or without syntax highlighting)?
It allows an easy retransition to a more efficient way of storing the flags, should the need arise to shift the balance between flash and RAM use.
you want us to live with this monstrous code on the offhand chance that in an unforseeable future the code might need just this change? With that same argument you could justify about everything, like making all vars float or whatever.
I can not see any good reason to trade off the former concise version of
Code: Select all
if (rcOptions[BOXGPSHOLD]) {
if !GPSModeHold & !GPSModeHome) {
GPSModeHold = 1;
against the code you proposed (see above to compare)
Now if we take a look at how other project handle this; from random linux drivers section they do the bitmasking in situ, so adopting their style would yield something like
Code: Select all
if (rcOptions[BOXGPSHOLD]) {
if !(GPSflags & GPS_HOLD_MODE) & !(GPSflags & GPS_HOME_MODE) {
GPSflags |= GPS_HOLD_MODE;
you get the idea. I do not advocate this, I do not know about effects on time and resources; just wanted to share another aspect.
Converting the current proposal in _shared to this could be done with some regexp.
Question remains: should we trade concise old style against any of the other flavours which bring
+ flags contain FLAG in their name
- source code increase
- reduced readability
- increased size
- no execution time improvement (or time penalty?)
Bottom line: does not look good. The old style has its merits still.
Take your pick