[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: [PATCH] Delayed notify - performance
From:       Patrick Julien <freak () codepimps ! org>
Date:       2004-03-08 10:49:57
Message-ID: 200403080549.57831.freak () codepimps ! org
[Download RAW message or body]

On March 8, 2004 02:40 am, Roger Larsson wrote:
> On Sunday 07 March 2004 14.39, Patrick Julien wrote:
> > Use a proper constant for kis_notify_limit...
>
> A constant can not be changed. So testers of the patch could
> not check how different values perform.

Declare the constant in a local namespace, I am worried the variable will be 
forgotten and will stay a global variable.  Doing

namespace {
	const Q_INT32 NOTIFY_LIMIT = XXX;
}

At the top of this file still allows for what you are doing and keeps the code 
clear.  Worst case scenario, if this is forgotten, it still leaves things in 
a desirable state.


>
> > Call m_dragNotifyTime.start() in the constructor.
>
> Why? The timing does not start here?

So that you can avoid checking a flag and calling start() vs. restart() in 
subsequent code.  Start is only for initialization, restart() is called when 
the timing is actually called.

> It starts when the button is pressed.

Hence, calling restart()

> I think this would only confuse things...
>
> > Then use
> > m_dragNotifyTime.restart() in timeToNotify() so that you fall within
> > documented behavior of QTime.
>
> Current code
> +    if (m_dragNotifyTime.elapsed() > kis_notify_limit)
> +    {
> +       m_dragNotifyTime.start();
>
> How?
> +    if (m_dragNotifyTime.restart() > kis_notify_limit)
> +    {

I did not state you needed to change elapsed to restart().
_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop
[prev in list] [next in list] [prev in thread] [next in thread]