<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/100336/">http://git.reviewboard.kde.org/r/100336/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 9th, 2011, 4 p.m., <b>Benjamin Poulain</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;">r-
That is saving a 64 bytes (the D-pointer of QString) for one frame, while making the code less readable.
The scope of the bool can be figured by the compiler. Instead, you are adding a function call.
This is not an optimization by any mean.</pre>
</blockquote>
<p>On January 9th, 2011, 4:09 p.m., <b>Rohan Garg</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;">Oh ok, but do you agree that i can remove the isOpened var with a !file.isOpen() ? </pre>
</blockquote>
<p>On January 9th, 2011, 4:39 p.m., <b>Benjamin Poulain</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;">> Oh ok, but do you agree that i can remove the isOpened var with a !file.isOpen() ?
Nope, that does not help.
And it is incorrect, in your patch, you end up never calling QFile::open() so the file won't work. The constructor does not open the file.</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;">Doh! Didnt think that through properly .... Discarding this review :)</pre>
<br />
<p>- Rohan</p>
<br />
<p>On January 9th, 2011, 3:35 p.m., Rohan Garg wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/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 rekonq.</div>
<div>By Rohan Garg.</div>
<p style="color: grey;"><i>Updated Jan. 9, 2011, 3:35 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;">Some code cleanup:
This patch removes 2 variables notfoundFilePath and isOpened because there are only 2 instances of these variables, one is the declaration and the other is their intialization. Also QFile has isOpen() which can be used to determine whether or not the file is open</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;">Compiles and when you open a invalid url displays "Couldn't open the rekonqinfo.html file" ( Which is a bug im working on )</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>src/webpage.cpp <span style="color: grey">(4705621)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100336/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>