> On Jan. 9, 2011, 4 p.m., Benjamin Poulain wrote:
> > 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.
> 
> Rohan Garg wrote:
>     Oh ok, but do you agree that i can remove the isOpened var with a 
> !file.isOpen() ?

> 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.


- Benjamin


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


On Jan. 9, 2011, 3:35 p.m., Rohan Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100336/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2011, 3:35 p.m.)
> 
> 
> Review request for rekonq.
> 
> 
> Summary
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/webpage.cpp 4705621 
> 
> Diff: http://git.reviewboard.kde.org/r/100336/diff
> 
> 
> Testing
> -------
> 
> Compiles and when you open a invalid url displays "Couldn't open the 
> rekonqinfo.html file" ( Which is a bug im working on )
> 
> 
> Thanks,
> 
> Rohan
> 
>

_______________________________________________
rekonq mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/rekonq

Reply via email to