[Kde-pim] Review Request: Use ItemRetriever as local var instead of inheriting from it in FetchHelper

[Bertjan Broeksema]

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



/trunk/kdesupport/akonadi/server/src/handler/fetchhelper.cpp
<http://reviewboard.kde.org/r/3819/#comment4763>

    This is the most important part of your change and I think this is indeed better than the inheritance solution.



/trunk/kdesupport/akonadi/server/src/storage/itemretriever.h
<http://reviewboard.kde.org/r/3819/#comment4764>

    Stated somewhere below too. But I think its *really* important to keep the Column postfix.
    
    Imo the code becomes less readable, i.e. harder to understand when you remove the prefix. Without the prefix it looks like if you're passing in the actual Id or Rid which is *not* the case.



/trunk/kdesupport/akonadi/server/src/storage/itemretriever.h
<http://reviewboard.kde.org/r/3819/#comment4760>

    Please remove the api doc here or change it. This is not valid anymore.



/trunk/kdesupport/akonadi/server/src/storage/itemretriever.cpp
<http://reviewboard.kde.org/r/3819/#comment4762>

    Please leave the Column prefix, this is actually what is passed in to the value call and not the pim item id itself. That should be clear from the code.


- Bertjan


On 2010-04-27 15:48:05, Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3819/
> -----------------------------------------------------------
> 
> (Updated 2010-04-27 15:48:05)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> First step towards optimizing the queries in ItemRetriever:
> 
> Let the ItemRetriever be used solely for making sure all items are in the database.
> To share code, make the generic*Query static and reuse it in the FetchHelper.
> 
> I think the latter can even be removed once I merge the queries in the ItemRetriever and the code sharing is gone.
> But to keep patches small...
> 
> 
> Diffs
> -----
> 
>   /trunk/kdesupport/akonadi/server/src/handler/fetchhelper.h 1118978 
>   /trunk/kdesupport/akonadi/server/src/handler/fetchhelper.cpp 1118978 
>   /trunk/kdesupport/akonadi/server/src/storage/itemretriever.h 1118978 
>   /trunk/kdesupport/akonadi/server/src/storage/itemretriever.cpp 1118978 
> 
> Diff: http://reviewboard.kde.org/r/3819/diff
> 
> 
> Testing
> -------
> 
> ran kmail & akonadiconsole on my mboxes, no difference.
> 
> 
> Thanks,
> 
> Milian
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/