[rekonq] Re: Review Request: Small Memory optimization

[Benjamin Poulain]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20110109/4b2ff7a9/attachment.htm