[Kde-pim] Review Request: Convert IMAP flags to internal Akonadi flags in the IMAP resource.

[Ingo Klöcker]

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


Using a hash for four different values is overkill. Using an if-else cascade or a QList<QPair<QByteArray,QByteArray>> with simple string comparisons will be way faster than computing qHash() and then doing a lookup in the hash table. A general rule of thumb is that simple lists are usually faster than hashes for less than 20 elements.

OTOH, this code is probably not really performance critical (is it?) so the usage of the QHashes isn't too bad. It might make sense to create overloads which take a QByteArray instead of QList<QByteArray>. Of course, this would mean that you cannot use method local statics for the hashes.


/branches/KDE/4.5/kdepim/runtime/resources/imap/imapresource.cpp
<http://reviewboard.kde.org/r/4760/#comment6490>

    Move the initialization to the corresponding methods and use QHash::insert( key, value ) instead of operator[] and operator=. You cannot init the static hashes in the c'tor because the c'tor does not even have to be called before one calls the static methods using the static hashes. Also each construction of an ImapResource will initialize the hashes again.



/branches/KDE/4.5/kdepim/runtime/resources/imap/imapresource.cpp
<http://reviewboard.kde.org/r/4760/#comment6491>

    const QByteArray& oldFlag
    
    Always use const T& for the variable in a foreach-loop to avoid unnecessary copying.



/branches/KDE/4.5/kdepim/runtime/resources/imap/imapresource.cpp
<http://reviewboard.kde.org/r/4760/#comment6492>

    ditto


- Ingo


On 2010-07-26 14:51:19, Leo Franchi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4760/
> -----------------------------------------------------------
> 
> (Updated 2010-07-26 14:51:19)
> 
> 
> Review request for KDE PIM and Kevin Ottens.
> 
> 
> Summary
> -------
> 
> As a result of the recent Akonadi server changes that have made the akonadi server case-sensitive for flags, the decision was made to store flags as all-uppercase. However, the flags from imap servers are not upper-cased, they are simply capitalized. This results in collection statistics always being incorrect (listing all items as unread).
> 
> After discussion with Tobias, a solution was proposed to do a mapping of KImap flags <--> Akonadi flags in the imap resource, as that is the interface between KImap and Akonadi. For now this just includes 4 standard imap server flags. I don't know if this is the best way to do this, but figured i'd throw it out there and start a discussion.
> 
>  Comments are appreciated :)
> 
> 
> Diffs
> -----
> 
>   /branches/KDE/4.5/kdepim/runtime/resources/imap/imapresource.h 1154848 
>   /branches/KDE/4.5/kdepim/runtime/resources/imap/imapresource.cpp 1154848 
> 
> Diff: http://reviewboard.kde.org/r/4760/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Leo
> 
>

_______________________________________________
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/