QTimer does not stop

[JonB]

@herrgross
The QTimer *timer needs to be a member variable of your class, so that it persists outside of whatever method it is created in. Usually in constructor, but you can also create it on demand in on_wanderButton_clicked() so long as you check whether the pointer already points to a created instance. In C++ you can also just create a QTimer timer (not pointer) in your class and use that, no creating/destroying needed.

[herrgross]

@JonB
wow!!! it works! thanxalot!!

[Pl45m4]

@herrgross

You state logic also seems not correct.
I can see no way how state is ever false. You set a new variable state to true when the button is clicked, after that it can never be false in the same slot. So even if you fixed the timer issue, this logic might not work as expected.

[JonB]

@Pl45m4
It's static, and he toggles its value in both/all paths each time button is clicked.

@herrgross
static is bad actually nowadays. Change that (state) also to a member variable, or retrieve state some other way (e.g. timer.isActive() should suffice for this code to toggle its started/stopped).

[Pl45m4]

@JonB said in QTimer does not stop:

@Pl45m4
It's static, and he toggles it's value in both/all paths each time button is clicked.

Ouh, missed the static keyword.

@herrgross Declaring a variable static in a slot every time the slot is called , is also not very good.

Edit: As @JonB said above, if you make it a member like your timer, you don't even need it to be static.

[herrgross]

@Pl45m4 @JonB
yes, if (!timer.isActive())
works as well and is shorter and no need to add in constructor....
thankyou

[aha_1980]

@Pl45m4 said in QTimer does not stop:

@herrgross Declaring a variable static in a slot every time the slot is called , is also not very good.

Sorry, I have to disagree. A static variable is just a global variable with limited scope (in this case: function scope) and initialization at the first time.

There is no performance penalty, and of course you can use static variable in a slot.

Regards

[SimonSchroeder]

@aha_1980 said in QTimer does not stop:

There is no performance penalty, and of course you can use static variable in a slot.

You are correct about that. However, just because you can use it, it doesn't mean you should. And in this specific case you should not. In case there are more instances of Wanderung in the future, the behavior becomes very confusing! And even if there won't be more instances of Wanderung this will start a really bad habit which is hard/annoying to debug & fix in other places. Most of the times in member functions/slots you will actually need a member variable and not a static variable (and a distant second is class variables before static variables inside member functions; static variables make a lot more sense in free-standing functions).

[aha_1980]

@SimonSchroeder You are absolutely right about that, but see that I wrote: is just a global variable, so everything about global variables apply.

I also fully agree that here a static variable is unneeded, my point was to justify "Declaring a variable static in a slot every time the slot is called , is also not very good", because that is not correct.

Regards

[Pl45m4]

@aha_1980 said in QTimer does not stop:

I also fully agree that here a static variable is unneeded, my point was to justify "Declaring a variable static in a slot every time the slot is called , is also not very good", because that is not correct.

Haha yes, but my statement was that it's not good. As we can see from @herrgross reply, it worked.
But this doesn't mean, that it's good style or a good practise to do so all the time. I wouldnt expect a static variable in a slot, especially when there are at least two better ways to achieve the same :)
(using the QTimer API, or adding some non-static member to check if timer needs to be stopped or started)