Review request for KSecretsService components

[Valentin Rusu]

On 11/10/2011 01:00 AM, Valentin Rusu wrote:
> On 11/10/2011 12:48 AM, Albert Astals Cid wrote:
>
>>
>>
>>>>     * createItem falls in the "let's use a bool instead of an enum
>>>>     because it>
>>>> just have two values" trap for the replace parameter
>>> Well, I don't agree. When presenting a new item to the collection, you
>>> should specify to replace existing item or not. Yes/No is boolean 
>>> for me.
>> That is the problem, that a boolean is Yes/No, and then you end up 
>> with a call
>> that says
>>   foo->createItem(label, attributes, mySecret, false);
>> And it seems you do not want to create the item :D while
>>   foo->createItem(label, attributes, mySecret, DoNotReplace);
>> is much more obvious.
>> More on my side at
>> http://wiki.qt-project.org/API_Design_Principles#The_Boolean_Parameter_Trap 
>>
>> and
>> http://ariya.ofilabs.com/2011/08/hall-of-api-shame-boolean-trap.html
> Ok,  I see. I'll change that.
Well, In fact I changed my mind, as the enum must be a member of 
Collection class, but required to be used in the create job and nested 
enums cannot be forward declared. So that'll require to move the enum 
outside of the Collection class, leading to even uglier code. So I'll 
stay with the boolean, wich also corresponds to the freedesktop.org dbus 
interfaces spec, btw.

-- 
Valentin Rusu (IRC valir, KDE vrusu)
KSecretsService (former KSecretService, KWallet replacement)