-
• #2
Good practice - I use - is:
- define (global) variables for all your watches, timeouts, and in intervals you plan to set
- give the name a suffix, like xyzWId or xyzWHndl for watch handles
- initialized them with
false
at declaration (optional, any falsy evaluating value will do: undefined, null, 0, "", false) - always use the variable when setting the Watch / Interval / Timeout
always use the following construct when clearing a Watch / Interval / Timeout - mostly I use a function or method of an object where I have a simplified invocation interface to make sure I do not misspell the variable name and other things. Most of the time there is more that has to go on at the same time with the particular
clear...()
, and that goes nicely in such a function or method.... and some times these things have even to go conditioned on the ...Id as well.var xgzWId = false; // ... // ... // ... xyzWid = setWatch(xyzFunction,pin,options); // ... // ... // ... if (xyzWId) { xyzWId = clearWatch(xyzWId); }
PS: clear...() returns undefined, which is falsy and does the expected job.
- define (global) variables for all your watches, timeouts, and in intervals you plan to set
-
• #3
Thanks for the input!
That snippet looks good, it's short, and I think still clear, after you learn that clearXX returns undefined! -
• #4
Forgot to add that 'mistakes' I noticed are setting up (especially)
setInterval()
. For example, the uploading onto the board already sets one (which 'never ever should be done), then the code is saved, and for some logic reason on power up thesetInterval()
is called again when power is cycled. Therefore, theabcIId
variable to hold theId
of the interval can be used too to prevent that. I the function is (the same and) with the same parameters, thesetup()
can be skipped. Otherwise, it has to be cleared and set again.NB: If just the time of the interval changes, you do not need to clear and setup the function again: you can just change the interval:
changeInterval(abcIId,newTimeInMS); // change rime of running abc interval
-
• #5
As you say this could easily be a source of potential confusion. I'll see how hard it is to change
clearXX
such that they error if called withundefined
-
• #6
Ok, just fixed - it'll be in 2v03.
But yes, I always try and do
if (interval) clearInterval(interval)
Hi!
Recently ran into an issue with
clearWatch
in library code. (This cost me couple of hours, so possibly over exaggerating this...)clearWatch()
andclearWatch(undefined)
clears all pin-watches. You can accidentally call it with undefined, if you don't check whether you have an actual watch Id, for example line 31-32 in DHT22.js.Because of the "reliability" of the DHT11 & 22 sensors and the 10 default retries in the libraries, these are dangerous for almost 6 seconds (550 ms timeout * 10). If you don't read the docs, you can run into a race condition where you start a conversion before all possible timeouts of the previous
read
had finished.If you use
setWatch
elsewhere, thatclearWatch(undefined)
just destroys you program without crash, and leads to some head scratching... Or leads to a memory leak, if you call.read()
too soon. It did leak slowly at 10 retries and 3 second read interval. At the end the setWatch 'count' was up to 50. But that's a different matter.Also
clearTimeout
andclearInterval
is the same.First, a suggestion:
What would be the preferred way to handle
clear***()
? Seen several ways in devices / modules code: I think it shouldn't be without checks and should set the 'watch' property to falsey, if there is one.For example there is this nice, but nontrivial looking example from the TouchRD code (
_
is "this", and_.w
is the watch Id):Or the more obvious
If there is a consensus, I'm willing to change the libraries, around dozen uses in JS files.
Does anyone think it would worth making
clearWatch(undefined)
invalid, and changing the clear-all call explicit (for example pass in-1
)? I know, it's a breaking change. Possibly write a warn to console?Or better just improve the libraries & docs?