[vlmc-devel] [PATCH] Fix QTimer running with its interval of 0

[Hugo Beauzée-Luyssen]

On 03/05/2016 02:08 PM, Yikai Lu wrote:
> The title says all. I fix it by setting a default interval value of 5 * 1000* 60 msec.
> ---
>  src/Project/AutomaticBackup.cpp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/Project/AutomaticBackup.cpp b/src/Project/AutomaticBackup.cpp
> index 5018a91..036f948 100644
> --- a/src/Project/AutomaticBackup.cpp
> +++ b/src/Project/AutomaticBackup.cpp
> @@ -31,6 +31,7 @@ AutomaticBackup::AutomaticBackup( Settings* vlmcSettings, QObject *parent )
>      , m_vlmcSettings( vlmcSettings )
>  {
>      m_timer = new QTimer( this );
> +    m_timer->setInterval( 5 * 1000 * 60 );
>  	SettingValue* autoBackup = m_vlmcSettings->createVar( SettingValue::Bool, "vlmc/AutomaticBackup", false,
>  									 QT_TRANSLATE_NOOP( "PreferenceWidget", "Automatic save" ),
>  									 QT_TRANSLATE_NOOP( "PreferenceWidget", "When this option is activated,"
> 

Hi,

This becomes useless as soon as setProject is removed.
While it's indeed correct that the timer would have been started with a
0 interval, it will behave correctly when the interaction are
originating from the variable signals.

However, 2 remarks on this class:
- It would be better to use setInterval rather than the start method, so
that we don't have to fetch the interval from the AutomaticBackup
callback (and also, we wouldn't have to check the
AutomaticBackupInterval from the AutomaticBackup callback)
- The interval setting should be clamped to reasonable values, as the
user can set it to 0 so far (if I read the code correctly)
There's no harm in checking that the value is non 0 from the slot as well.

Thanks!

Regards,

-- 
Hugo Beauzée-Luyssen
www.beauzee.fr