On Sunday, 2012-10-28, Martin Klapetek wrote: > On Sat, Oct 27, 2012 at 6:05 PM, Kevin Krammer <[email protected]> wrote:
> > - FacebookJob(QString, QString) calls setCapabilities, > > FacebookJob(QString) does not > > > > - Is the FacebookJob(QString) constructor really needed. It seems all > > jobs need a path > > This is used (only) from FacebookGetJob, assumingly for querying multiple > items, where just the ids are specified and no path is needed. I'll see > what can be done about this. Even FacebookGetJob and its subclasses set a path. FacebookGetIdJob for multiple IDs sets the query as query item instead of encoding it in the path itself, but it nevertheless needs "/" as path. So path is always needed, the base class can always require it. > > - the *Info classes seem to be normal data classes, IMHO they don't need > > to be > > QObjects but rather "value types" > > This is for the parsing purposes - the library uses QJson parser/mapper, > which automagically maps the received json data to qobjects, otherwise > there would have to be manual parsing everywhere (and the facebook jsons > are huge), which means more code, more error possibilities, more > maintaining requirement and worse readability (compared to two lines QJson > mapper). So I'd like to leave this one as is. I haven't had a look at the QJson library internals (yet), but from its usage it looks like that it is only using instances of those QObject classes to provide a convenient mapper of map keys to conversion functions (the property setters). This would make them an internal implementation detail, something more convenient than manually writing a mapping of string to function pointer but also just private. As I said I'll have a look into QJson, but unless I am gravely mistaken it only needs such QObjects as a generic accessor API, not as the actual data object. Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring
signature.asc
Description: This is a digitally signed message part.
