[KParts] patch: don't emit urlChanged, when url is the same

[David Faure]

On Tuesday 24 December 2013 13:50:29 Michal Humpula wrote:
> On Tuesday 24 of December 2013 13:22:05 David Faure wrote:
> > On Tuesday 24 December 2013 12:30:56 Michal Humpula wrote:
> > > Hi there,
> > > 
> > > as suggested without fully functional reviewboard, here is the little
> > > patch
> > > for discussion.
> > > 
> > > Emit urlChanged only when actually changing url trough setUrl. The
> > > drawback
> > > of this is that now the fast assignment is replaced by possibly
> > > expensive
> > > string comparison.
> > 
> > That's ok, this method is typically not called 10000 times per second.
> > 
> > > On the other hand, when the urls are the same, the emit
> > > wont fire and whole bunch of potentially expensive operations connected
> > > to
> > > that signal will not happen.
> > 
> > Yep. The patch makes a lot of sense, please commit it - at least to
> > frameworks. Do you also need it in kdelibs master?
> 
> Nope, no need. Just to be sure, pushing to master branch of
> 
> git at git.kde.org:kparts

Yes.

Please fix the coding style first: the '{' belongs on the same line as the 
if().

The code in frameworks/* should be very consistent now, since I reformatted it 
all with astyle. So no excuse anymore for inconsistent coding style in new 
code :)

> But, if I have a green light, then I would like to change the second
> callsite of urlChanged() as well. Please, see attached patch. The reasoning
> is the same as with setUrl.

Coding style: please use { ... } even for one-line statements.

Looks OK otherwise.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5