<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105130/">http://git.reviewboard.kde.org/r/105130/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 2nd, 2012, 6:58 a.m., <b>Dominik Cermak</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On June 2nd, 2012, 11:39 a.m., <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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! :)</pre>
</blockquote>
<p>On June 2nd, 2012, 11:42 a.m., <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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".</pre>
</blockquote>
<p>On June 2nd, 2012, 11:59 a.m., <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not saying _super_ intense, I'm saying "let's not do it intense if we don't have to" ;)</pre>
<br />
<p>- Martin</p>
<br />
<p>On June 1st, 2012, 8:23 p.m., Dominik Cermak wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy.</div>
<div>By Dominik Cermak.</div>
<p style="color: grey;"><i>Updated June 1, 2012, 8:23 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Now the counters work as expected.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=300956">300956</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>KTp/Models/accounts-filter-model.h <span style="color: grey">(9ed824a1e9a970e33b07343ae9143734857820c8)</span></li>
<li>KTp/Models/accounts-filter-model.cpp <span style="color: grey">(c17b3359178ffceca5ae815d41008058218f3bf5)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105130/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>