On 11/10/2011 12:48 AM, Albert Astals Cid wrote:
A Dijous, 10 de novembre de 2011, Valentin Rusu vàreu escriure:
Hello Albert,
Thanks for the thourough review.
On 11/09/2011 03:26 PM, Albert Astals Cid wrote:
This is scary, last time i used kwallet, i had to add a single line, and
now there are like a billion of classes?
Welcome to the asynchronous jobs world. I had the same opinion at start.
And, to be honest, that's why it takes so long to implement, as I'm not
working on it only during my spare time.
Are you saying you don't have anything as simple as this in the new API?
QString walletName = KWallet::Wallet::NetworkWallet();
KWallet::Wallet *wallet = KWallet::Wallet::openWallet(walletName, parentwid);
if ( !wallet->hasFolder( "KPdf" ) )
wallet->createFolder( "KPdf" );
wallet->setFolder( "KPdf" );
wallet->readPassword( walletKey, retrievedPass )
Yes, your code isn't asynchrounous ! (boo)
Well, I don't fully agree with this point of view, but in Randa they
managed to convince me about the benefits.
Take a look inside kwallet.cpp class to see how this new API should be used.
Another example is the ksecrets API inside kdereview/ksecrets repository.
* 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.
Of course that is a minor thing and won't complain too much if you decide to
disagree with me ;-)
Albert
--
Valentin Rusu (IRC valir, KDE vrusu)
KSecretsService (former KSecretService, KWallet replacement)
