Re: [PATCH_v3 1/4] GAtPPP: Add ACFC option support

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 875 bytes --]

Hi Marcel,

On 06/28/2011 08:24 PM, Marcel Holtmann wrote:
> Hi Denis,
> 
>>> @@ -658,6 +703,11 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote,
>>>  	ipcp_set_server_info(ppp->ipcp, r, d1, d2);
>>>  }
>>>  
>>> +void g_at_ppp_set_acfc_enabled(GAtPPP *ppp)
>>
>> I really do want the signature to be g_at_ppp_set_acfc_enabled(GAtPPP
>> *ppp, gboolean enabled)
>>
>> There are cases where we might re-use the PPP object with different
>> parameters.
> 
> what is wrong with just g_at_ppp_set_acfc(GAtPPP *ppp, gboolean enabled)
> as function name. Duplicating enabled in the function name and as
> parameter seems to be bit odd.

For APIs I generally prefer:

_set_foo() when foo is a 'thing', e.g int/string/struct/etc

and _set_foo_enabled() when foo is on/off as that intent is clearer when
reading the code.

Regards,
-Denis