[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Moving libkfacebook to extragear
From: Kevin Krammer <krammer () kde ! org>
Date: 2012-10-29 7:28:31
Message-ID: 201210290828.36294.krammer () kde ! org
[Download RAW message or body]
On Monday, 2012-10-29, Martin Klapetek wrote:
> On Sun, Oct 28, 2012 at 8:03 PM, Kevin Krammer <krammer@kde.org> 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" (application/pgp-signature)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic