Review Request 108992: Simple optimizations in SignalPlotter

[Aaron J. Seigo]

> On Feb. 18, 2013, 1:39 a.m., Aleix Pol Gonzalez wrote:
> > I don't see loosening the variables' scope as a codebase improvement. Mostly otherwise.
> > 
> > Also I'd like to know how you measured this 5% of improvement, which either way I'm unsure if it's worth it considering that this patch makes everything global, now.
> 
> Raul Fernandes wrote:
>     I think it is the worst response that I can have.
>     Never in my entire life I saw anyone that complains about creating classes outside loops looses the scope because it is one of the most basic forms of optimizing the code. Creating classes inside loops is a great waste of resources.
>     That why KDE4 is still bloat and slow.
>     Is is so true that KDE3 is still alive.
>     I think some developers should learn how to write better and fast code and avoid some commentaries like this or "This is only one more full update. Who cares??" (I saw this insanity in some place in KDE's code).
>     This patch is one of those that I should not put in review because it is so basic, but I do because I don't want to commit anything without approval.

Raul, thanks for the patch. We appreciate contributions such as these, particularly ones that work on improving what exists .. and this bit of code definitely could use it.

However, we do not interact with each other in the way you did in your comment here. We try to show each other respect and understanding. Not only do we find more enjoyment and team spirit this way, but it prevents people from treating us the same way. There are issues with your patch which I cover in my review, but I don't suggest you ought to learn how to write better code or otherwise insult you. What would that achieve, besides getting on your nerves? Nothing.

You did not answer Aleix's question about how you measured the improvement, which is a very valid question. The least one can do is respect an honest question with an honest answer, yes?

Our code of conduct describes our commitments to each other in these things: http://www.kde.org/code-of-conduct/

I hope you can receive this comment in the constructive manner which I intend it, and look forward to more of your patches in the future. :)


- Aaron J.


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


On Feb. 17, 2013, 12:57 p.m., Raul Fernandes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108992/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2013, 12:57 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> - create variables and classes outside the loops
> - reserve space in QList if we know already how many items will be added (avoid unnecessary reallocations)
> - use const_iterator when possible
> - remove a useless call (p->setPen(Qt::NoPen) - it will be set latter before be used)
> - avoid multiplications (x3, x2, x1 and x0)
> 
> 
> Diffs
> -----
> 
>   plasma/widgets/signalplotter.cpp 8e9e294 
> 
> Diff: http://git.reviewboard.kde.org/r/108992/diff/
> 
> 
> Testing
> -------
> 
> I have tested with KDE 4.10 with no problems.
> I have seen a improvement of about 5% in drawPlots() function, the most expensive function in painting.
> 
> 
> Thanks,
> 
> Raul Fernandes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130222/3cf8db59/attachment-0001.html>