Review Request: Respect filters for counters

Martin Klapetek martin.klapetek at gmail.com
Sat Jun 2 12:56:27 UTC 2012



> On June 2, 2012, 6:58 a.m., Dominik Cermak wrote:
> > Forget about diff 2, it's failing in one case: 'Show offline users'. If this is selected it doesn't show the online count anymore, so we are back at diff 1.
> 
> Martin Klapetek wrote:
>     Thinking now about it - it should definitely not be in the data(), because this can (and does) be called looots of time per second, so that would count this stuff again and again. Ideally we should have a method in the filter model, that recounts either online or the total count, have a class variable that stores this count and this variables are then returned in the data() while the count methods are triggered only when there is actual change (ie. on contact going offline/online or on contact being added/blocked). So it'll probably get more complex.
>     
>     As for the rowCount() - I think adding some ifs and using it when appropriate could work (for example only count it manually when we're showing online users - and it could be even so clever, that when you change the type of filter, the recount is not triggered until there is an actual need for recount [user going offline], but that's a bit too complex for now). Also in the state when we're showing /all/ contacts, it could use sourceModel()->rowCount(index), because these are already counted elsewhere, so no need to count it on our own again. 
>     
>     Nevertheless, good start! :)
> 
> Martin Klapetek wrote:
>     Crap, my english is still clouded by yesterday's beer. The second paragraph in the first bracket should of course say "when we're showing offline users".
> 
> David Edmundson wrote:
>     data() gets called a lot, but not that often for the role TotalUsersCountRole, once per repaint. and this is simply doing some counting and filtering it's not _super_ intense.

I'm not saying _super_ intense, I'm saying "let's not do it intense if we don't have to" ;)


- Martin


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


On June 1, 2012, 8:23 p.m., Dominik Cermak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105130/
> -----------------------------------------------------------
> 
> (Updated June 1, 2012, 8:23 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> I tried to solve it on filter model level. Here is what I've done so far, it's not finished yet but I want to show it so you can speak up if it's totally (or partly) wrong, so I don't waste time. I still have to find out what I can remove from the other code now.
> 
> Also it's not so nice to have so much in data(), so if someone has an idea how to make this more elegant, I'm open for suggestions.
> 
> 
> This addresses bug 300956.
>     http://bugs.kde.org/show_bug.cgi?id=300956
> 
> 
> Diffs
> -----
> 
>   KTp/Models/accounts-filter-model.h 9ed824a1e9a970e33b07343ae9143734857820c8 
>   KTp/Models/accounts-filter-model.cpp c17b3359178ffceca5ae815d41008058218f3bf5 
> 
> Diff: http://git.reviewboard.kde.org/r/105130/diff/
> 
> 
> Testing
> -------
> 
> Now the counters work as expected.
> 
> 
> Thanks,
> 
> Dominik Cermak
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120602/8b2fa77f/attachment.html>


More information about the KDE-Telepathy mailing list