Re: Coding style question

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Wed, 22 Aug 2012 13:00:37 +1200

On 22.08.2012 03:48, Alex Rousskov wrote:
> On 08/21/2012 09:26 AM, Kinkie wrote:
>> On Tue, Aug 21, 2012 at 4:47 PM, Alex Rousskov
>> <rousskov_at_measurement-factory.com> wrote:
>>> On 08/21/2012 12:19 AM, Kinkie wrote:
>>>>> .h file:
>>>>>> #if OPTIONAL_FEATURE
>>>>>> extern void someFunction();
>>>>>> #else
>>>>>> // #define someFunction() // NOP
>>>>>> // or:
>>>>>> // static inline someFunction() {/* NOP */}
>>>>>>
>>>>>> #endif
>>>>>>
>>>>>> .cc file:
>>>>>> #if OPTIONAL_FEATURE
>>>>>> void someFunction()
>>>>>> {
>>>>>> //code
>>>>>> }
>>>>>> #endif
>>>>>>
>>>>>> client code:
>>>>>> someFunction(arg);
>>>> If any, I'd go for static inline (or just inline, after all the
>>>> namespace is already polluted).
>>>> Is the cost of the extra function call worth the decreased
>>>> readibility?
>>>
>>> No statics or #defines in headers unless really necessary, please.
>>>
>>> Use inline. Inline has the same effect on the namespace as static
>>> but
>>> the call to an [empty] inline function is more likely to be removed
>>> by
>>> the compiler. If done right, there will be no runtime cost in most
>>> cases.
>>>
>>> Please be careful about function arguments though. They or related
>>> conversion operators are sometimes unavailable outside of
>>> OPTIONAL_FEATURE protection.
>>>
>>> You may have to create a shadow hierarchy of types and names to use
>>> this
>>> approach in some cases (some recent Squid code does things like
>>> that;
>>> see src/ipc/AtomicWord.h, for example).
>>>
>>> This NullObject-like approach works best for commonly used things.
>>> If we
>>> are talking about a single obscure function call which requires
>>> #inclusion of a complex header, then it may be best to leave it
>>> (and its
>>> #include) inside #ifdef guards to minimize complications.
>>
>> I'm thinkin about applying some judgement and the Pareto principle:
>> if
>> we can reduce 80% of #if USE_STUFF we have got a good result
>> already.
>>
>> For instance:
>> in protos.h
>> #if USE_WCCP
>> void wccpInit(void);
>> #endif
>>
>> in wccp.cc
>> #if USE_WCCP
>> // stuff
>> void
>> wccpInit(void)
>> {
>> // code
>> }
>> // stuff
>> #endif
>>
>> in main.cc:
>> #if USE_WCCP
>> wccpInit();
>> #endif
>>
>>
>> This is a very simple example, and also a very typical one.
>> I'd like to turn it into:
>>
>> wccp.h:
>> /** initialize wccp
>> * \note it's a NOP unless USE_WCCP is defined
>> */
>> void wccpInit(void);
>>
>> wccp.cc:
>> #if USE_WCCP
>> // stuff
>> void
>> wccpInit(void)
>> {
>> // code
>> }
>> // stuff
>> #else
>> void wccpint(void) {}
>> #endif
>>
>> main.cc:
>> wccpinit();
>>
>>
>> Alternatively (less readability and less runtime overhead),
>> wccp.h:
>> #if USE_WCCP
>> void wccpInit(void);
>> #else
>> inline void wccpInit(void) {}
>> #endif
>> //everything else as previous example
>
> Either one is OK in this case and seems to have similar readability
> IMO.
> Pick whatever style you prefer and try to be consistent.
>
> However, for this specific example, one should probably use
> RegistryRunners instead of making main.cc to be aware of WCCP.
>
>
>> // but in this case, where do we put doxyigen-docs? Before the #if
>> USE_WCCP?

I think; Inside the #if, next to the real definition or in the .cc next
to the real code.

The stub definition should only ever be used in user builds which want
to omit the feature. Doxygen should always have those #if pre-defined in
its config to make them FOO=1 so it can parse the real definition and
produce linked-from/links-to documentation accurately.

>
> Ideally, this alternative requires different docs for USE_WCCP and
> !USE_WCCP cases (since both are used). If that is not an option,
> before
> #if is probably better than inside #if. I am not an expert on this
> though.

IIRC doxygen was a bit braindead and links it to the #if macro itself
in this case. Since it documents both compiler AND precompiler symbols -
differentiating them when two are in sequence is difficult for it.

Amos
Received on Wed Aug 22 2012 - 01:01:05 MDT

This archive was generated by hypermail 2.2.0 : Wed Aug 22 2012 - 12:00:13 MDT