Review Request 111636: Port away from kde_file.h in AuthInfo (KIO)

[Kevin Ottens]

> On July 23, 2013, 9:42 a.m., Kevin Ottens wrote:
> > staging/kio/src/core/authinfo.h, line 375
> > <http://git.reviewboard.kde.org/r/111636/diff/1/?file=172717#file172717line375>
> >
> >     Should be "const QString &"
> 
> David Gil Oliva wrote:
>     I think you didn't notice something important: the variable line gets modified in the method, so no const possible :-)
>     
>     Passing the variables by value was intentional because that way the parsing done inside the method doesn't disturb the parsing done outside.
> 
> Kevin Ottens wrote:
>     Well, it means introducing a local variable obviously.
> 
> David Gil Oliva wrote:
>     Maybe it's because I don't have much experience programming, but I don't think it's so obvious. 
>     
>     AFAIK, it's not mandatory to pass variables by const reference. Perhaps the key question is what's the difference between, on the one hand, passing a variable by const reference and create a local variable and, on the other hand, simply passing it by value. I really need some reason (be it style or clarity or performance or anything else) to be convinced that your proposal is better than mine. Otherwise, I can't understand the problem :-)

It's a coding style issue really (that's what the kdelibs style mandates).

It's generally better performance wise ("avoid premature pessimization"), although in this case it'll be the same performance wise. Still it has the advantage IMO of making the copy explicit and not implicit.


- Kevin


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


On July 21, 2013, 11:41 p.m., David Gil Oliva wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111636/
> -----------------------------------------------------------
> 
> (Updated July 21, 2013, 11:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Port away from kde_file.h in AuthInfo (KIO)
> 
> I have tried not to touch much code, but I finally rewrote some parts to make them easier to understand.
> 
> 
> Diffs
> -----
> 
>   staging/kio/src/core/authinfo.h d6415b2f2e9ccec7c3e046f569fb44dbbc879d6b 
>   staging/kio/src/core/authinfo.cpp 65ebacf84e989f19f1b896c596a6b24185c67447 
> 
> Diff: http://git.reviewboard.kde.org/r/111636/diff/
> 
> 
> Testing
> -------
> 
> It builds. I have tested the part of the code not related to loginMap with a little program and .netrc sample files, to check whether it correctly parses the information.
> 
> 
> Thanks,
> 
> David Gil Oliva
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130724/d7a5399d/attachment.html>