New authorization library moved to kdereview

[Aurélien Gâteau]

Le jeudi 23 juillet 2009 15:10:28, Nicola Gigante a écrit :

> > * You have functions that return references (e.g. ActionReply.h), if
> > somewhen
> > your internal storage changes you won't be able of returning
> > references ending
> > up in a BC problem, you should avoid/remove them
>
> Actually, that's an issue, but the two methods in question,
> Action::arguments() and ActionReply::data(),
> are meant to be used to directly access the QVariantMap object. If I
> remove the const, I wouldn't be able to
> write something like:
>
> action.arguments()["key"] = value;
>
> which is very confortable if you have to only set one or two keys. If
> I remove the reference (and add a setter method then),
> the library user has to write:
>
> QVariantMap args;
> args["key"] = value;
> action.setArguments(args);
>
> That would be, in my opinion, an ugly API.
> There shouldn't be any changes to the internal storage of that
> QVariantMap object, because its only purpose is to be sent directly to
> the helper and back again.
>
> Anybody else has suggestions about this point?

You could provide a setArguments() method and a addArgument() convenience 
method. The code would then be:

action.addArgument("key", value);

>
> > * static ActionReply deserialize(QByteArray data); should probably
> > get a
> > const &
>
> Probably. I've not used const & because I directly pass the byte array
> to the QDataStream
> constructor that discards the const qualifier. If I add const &, how
> do I solve the issue? QByteArray is implicitly shared anyway..

Make a copy of the data array inside your method. The behavior will still be 
the same as now, but you will have the ability to change it later, without 
breaking the API.

Aurlien