iup-rust
iup-rust copied to clipboard
Timer Guard is much too easy to mis-use
I suggest removing the Self return type from run, stop and set_time. Or changing it to &mut Self so chaining still works but you don't accidentally unwrap the Guard while still having the guard.
Even better would probably be to remove the Guard alltogether and implement Drop for Timer. That would remove the possibility for others to stop/start the timer. So another way would be to simply use Rc instead of Guard.
It was indeed a hard choice when I was designing the interface for those resources that aren't attached to anything. I don't recall why this choice in special was made (I could have documented it, eh, or perhaps a example/ would help me recall).
In any case, I'm neutral about this, might be a good idea, needs a study about it again. You can do so yourself and open a PR with the changes if you wish (with examples even) :)
Somethings to keep in mind:
- Clipboard and timer, for consistency, should have a similar interface. So, not only make the timer easy to use correctly but the clipboard too. Remember that a image sometimes may need to be guarded too.
- There should be a way to guard/unguard the resource, so you can send/receive to C or Lua [or LED]. I think this feature of cross-language is pretty neat (forgot to add this to features list hehe). Plus when someone wants to unguard a image to add to a widget.
- If I remember anything else I'll add here.
I always attach my Timers to the Display. I thought that was necessary and they don't mess with the layout anyway.
- Will they be properly destroyed if they are attached?
- How would I detect if they are not properly destroyed?
- Is the only problem with not destroying them a memory leak? Or can this somehow cause UB?
Hum, how do you attach it? Since the Node trait is required to append something into a container and the Timer doesn't implement it.
The IUP documentation states that it should be destroyed manually. Doubt they ever intended timer to be attached in anyway with containers.
You could try the following: Associate a handle name with the timer (add_handle_name), attach to the Display (Dialog you mean?), destroy this dialog, and then debug print Handle::from_name to see if the timer handle has been destroyed together with the dialog.
Yeah, I think it's all about memory leaking.