On Monday, 2012-10-29, Martin Klapetek wrote: > On Sun, Oct 28, 2012 at 8:03 PM, Kevin Krammer <[email protected]> wrote: > > > 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. > > Thanks. I fixed all the issues you pointed out except this one.
I think you can remove m_accessToken, m_path and m_queryItems from FacebookJob. Access token and path are already set on m_url and addQueryItem can be implemented to just call m_url.addQueryItem(). Also, virtual void start() = 0 is already part of KJob's API, so not needed again. Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring
signature.asc
Description: This is a digitally signed message part.
