A style question for buga (and maybe Alex)

This forum is dedicated to software development related to MultiWii.
It is not the right place to submit a setup problem.
Software download
Post Reply
Danal
Posts: 137
Joined: Tue Oct 18, 2011 5:15 pm

A style question for buga (and maybe Alex)

Post by Danal »

Buga,

Style question for you... and this is style, no right or wrong answers... :D

When you integrated the BTSERIAL code and the SPEKTRUM code, it needed the #ifdefs that surround the call to computerc to suppress that call for both BTSERIAL and SPEKTRUM. You came up with a solution, namely the NOCOMPUTERC definition. The NOCOMPUTERC define itself is automatically defined (or not) depending on the presences/absences of the BTSERIAL and SPEKTRUM defines. This works, and works great.

I have two style questions, the second of which becomes moot if we make the adjustment I'm going to propose for the first... so I'll hold the second until you respond. The first question is this:

Throughout the code there are many areas where a given #ifdef needs to turn on (or off) code for multiple settings. In all cases, the #ifdef is structured something like

Code: Select all

 #if !(defined(SPEKTRUM) || defined(BTSERIAL))
Although, to be clear, most examples don't have the ! and the extra parentheses. Here is a specific example from the 'output' tab

Code: Select all

  #if defined(LOG_VALUES) || (POWERMETER == 1)
    logMotorsPower();
  #endif


This accomplishes the same thing. From a style perspective, in my opinion, this makes the code easier to read. The exact things that turn that particular #ifdef on or off are right there to see. When using an abstracted define like NOCOMPUTERC, the reader of the code must look in another module to find out what that means, and what define(s) turn it on or off.


Well, I've used the word "style" about 20 times already, but I want to repeat: Either way will work. I'm asking here to have this snippet of code follow the style already established in the rest of Multiwii code. In fact, I feel so strongly about this that I'm going to make the change in shared in a moment. Feel free to regress it out if you strongly disagree.

And, Alex, thoughts?

User avatar
LuisFCorreia
Posts: 32
Joined: Sat Oct 01, 2011 7:04 pm
Location: Portugal
Contact:

Re: A style question for buga (and maybe Alex)

Post by LuisFCorreia »

Hi Danal,

as long as it works, either way is fine be me.

Let us wait for Alex to decide, but I really guess that the code style used throughout the whole code is the way to go.


Luis Correia aka Buga :)

Alexinparis
Posts: 1630
Joined: Wed Jan 19, 2011 9:07 pm

Re: A style question for buga (and maybe Alex)

Post by Alexinparis »

Hi Danal,
it's fine for me.
I understand your point

Danal wrote:Buga,

Style question for you... and this is style, no right or wrong answers... :D

When you integrated the BTSERIAL code and the SPEKTRUM code, it needed the #ifdefs that surround the call to computerc to suppress that call for both BTSERIAL and SPEKTRUM. You came up with a solution, namely the NOCOMPUTERC definition. The NOCOMPUTERC define itself is automatically defined (or not) depending on the presences/absences of the BTSERIAL and SPEKTRUM defines. This works, and works great.

I have two style questions, the second of which becomes moot if we make the adjustment I'm going to propose for the first... so I'll hold the second until you respond. The first question is this:

Throughout the code there are many areas where a given #ifdef needs to turn on (or off) code for multiple settings. In all cases, the #ifdef is structured something like

Code: Select all

 #if !(defined(SPEKTRUM) || defined(BTSERIAL))
Although, to be clear, most examples don't have the ! and the extra parentheses. Here is a specific example from the 'output' tab

Code: Select all

  #if defined(LOG_VALUES) || (POWERMETER == 1)
    logMotorsPower();
  #endif


This accomplishes the same thing. From a style perspective, in my opinion, this makes the code easier to read. The exact things that turn that particular #ifdef on or off are right there to see. When using an abstracted define like NOCOMPUTERC, the reader of the code must look in another module to find out what that means, and what define(s) turn it on or off.


Well, I've used the word "style" about 20 times already, but I want to repeat: Either way will work. I'm asking here to have this snippet of code follow the style already established in the rest of Multiwii code. In fact, I feel so strongly about this that I'm going to make the change in shared in a moment. Feel free to regress it out if you strongly disagree.

And, Alex, thoughts?

Post Reply