<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/101899/">http://git.reviewboard.kde.org/r/101899/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 11th, 2011, 11:10 a.m., <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101899/diff/1/?file=26605#file26605line115" style="color: black; font-weight: bold; text-decoration: underline;">app/telepathy-chat-ui.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TelepathyChatUi::handleChannels(const Tp::MethodInvocationContextPtr<> & context,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">111</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="kt">int</span> <span class="n">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span> <span class="n">i</span> <span class="o"><</span> <span class="n">m_chatWindows</span><span class="p">.</span><span class="n">count</span><span class="p">()</span> <span class="o">&&</span> <span class="o">!</span><span class="n">tabFound</span><span class="p">;</span> <span class="o">++</span><span class="n">i</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">personally I much prefer "foreach ". I think it's a lot easier to read.
and you can just use 'break' instead of this "&& !found".
I wouldn't say your way is any better than what was there before, but not any worse either.
Meh.
</pre>
</blockquote>
<p>On July 11th, 2011, 6:40 p.m., <b>Francesco Nwokeka</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;">foreach creates a copy of every item contained in the vector plus the previous iteration cycled through the whole vector.
We don't need to cycle through the rest of the vector once the chatwindow has been found.
I don't like to use "break" in cycles because they don't match a condition for which we're supposed to exit the cycle.
I also found this on iterations with for & foreach http://www.codeproject.com/KB/cs/foreach.aspx</pre>
</blockquote>
<p>On July 11th, 2011, 7:12 p.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;">- That link is talking about C#.
- foreach here is a Qt addition to the C++ language. Whilst it (theoretically) does copy the list, thanks to implicit sharing that is just incrementing a ref count in the QList object.
- using const references saves it copying each item.
</pre>
</blockquote>
<p>On July 11th, 2011, 7:32 p.m., <b>Francesco Nwokeka</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;">ok, so a const reference solves the preformance issue. I found this post you guys might find interesting
http://labs.qt.nokia.com/2009/01/23/iterating-efficiently/
towards the end he says what you say. K, modifying the patch</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">But I'm still uncomfortable with the "break"
</pre>
<br />
<p>- Francesco</p>
<br />
<p>On July 9th, 2011, 12:28 p.m., Francesco Nwokeka 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 Francesco Nwokeka.</div>
<p style="color: grey;"><i>Updated July 9, 2011, 12:28 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;">This patch consists in a tidy-up of the previous patch and the elimination of unused methods, logic and the tidy up of some methods and functionality.
Added comments to code as well.
The aim of this patch is to keep the code as tidy and simple as possible.</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;">Created more than one chat forcing the creation in one window, detached, closed, reopened and all went well.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>app/chat-tab.h <span style="color: grey">(408bb91)</span></li>
<li>app/chat-tab.cpp <span style="color: grey">(018868e)</span></li>
<li>app/chat-window.h <span style="color: grey">(a46e41b)</span></li>
<li>app/chat-window.cpp <span style="color: grey">(28905fa)</span></li>
<li>app/telepathy-chat-ui.h <span style="color: grey">(ad6809f)</span></li>
<li>app/telepathy-chat-ui.cpp <span style="color: grey">(71ac4ee)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101899/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>