Review Request 109481: Simplify the Nepomuk data extraction Code

[Martin Klapetek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109481/#review29213
-----------------------------------------------------------

Ship it!


Great! Couple style nitpicks and questions below.


src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21800>

    QString& value -> QString &value



src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21801>

    I don't understand this check - if it's a multi hash, why do we allow only one item?



src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21802>

    We can speed it up by 
    
    if (d->data.remove(role)) {
        emitDataChanged();
    }



src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21803>

    Yes.



src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21804>

    Remove this optional



src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21805>

    Model* m -> Model *m



src/contact-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21806>

    I think we should just clean the has completely before this step and then discard any checks for it



src/person-item.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21807>

    Item* item -> Item *item



src/persons-model.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21808>

    QString& bName -> QString &bName



src/persons-model.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21809>

    Add {}



src/persons-model.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21810>

    {}



src/persons-model.cpp
<http://git.reviewboard.kde.org/r/109481/#comment21811>

    Item* item -> Item *item


- Martin Klapetek


On March 14, 2013, 1:30 p.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109481/
> -----------------------------------------------------------
> 
> (Updated March 14, 2013, 1:30 p.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Description
> -------
> 
>     Simplify the Nepomuk data extraction Code
>     
>     Instead of using the Nepomuk property URIs as keys in ContactItem we now
>     use the roles instead. This simplifies the data extraction part from the
>     model as we can keep a direct mapping in the query (Binding Name => Role)
>     
>     Additionally, the ContactItem class could also load the data on its own
>     by using the Nepomuk Resource class. It now uses the same query as the
>     PersonsModel and fetches all the data in one go. This is much faster and
>     simpler. Though, it results in a little bit of code duplication.
>     
>     And finally, the ResourceWatcher now reloads the contacts which are
>     modified instead of just changing those specific properties. This is
>     done because in many cases such as - ImAccounts, Emails, Photos, and
>     ContactGroups, one needs to query the Nepomuk DB in order to fetch more
>     information.
> 
> 
> Diffs
> -----
> 
>   src/contact-item.h e970a6d 
>   src/contact-item.cpp e6b8277 
>   src/person-item.cpp 7a8f520 
>   src/persons-model.cpp 4e82688 
>   src/resource-watcher-service.h 8484162 
>   src/resource-watcher-service.cpp 6aafb83 
>   src/tests/duplicatestest.cpp 3ebde5a 
> 
> Diff: http://git.reviewboard.kde.org/r/109481/diff/
> 
> 
> Testing
> -------
> 
> Tested by using the ktp-contactlist. It looks the same in both the cases. 
> 
> Also tested the ResourceWatcher by -
> * manually changing an imNickName of one contact and it got updated.
> * Adding a nco:photo for a nco:PersonContact and it also got updated.
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130314/b723d09f/attachment-0001.html>