haxetink / tink_core

Core utilities
https://haxetink.github.io/tink_core
MIT License
117 stars 33 forks source link

Add Future.delay() #98

Closed kevinresol closed 5 years ago

kevinresol commented 5 years ago

... based on haxe.Timer

gene-pavlovsky commented 5 years ago

I quite often use this exact utility function. The only thing I'm not sure about: is it better to include it in Future or have it in a separate utils class? I think in a work-related project we have this in TimerUtil class, actually. But it is a very commonly used primitive, useful to build other things upon, so perhaps it makes sense to put it in the core class.

gene-pavlovsky commented 5 years ago

It also would be lovely to add a test :)

back2dos commented 5 years ago

As stated a few days back on gitter, I'm thinking that this might be nicer:

 @:noUsing static public function delay<T>(ms:Int, value:Lazy<T>):Future<T> 
    return Future.async(function(cb) haxe.Timer.delay(function () cb(value.get()), ms));

Then the Future.async(123) case is written as Future.async(123, Noise) which I'd say is a reasonable amount of overhead. OTOH if we go with the current signature, then instead of writing Future.async(123, "foo") you have to write Future.async(123).map(_ -> "foo") - if you ask me it's a bit too much noise for not having Noise ^^

Then again, I'm splitting hair here, so if you prefer keeping the signature as is, it's fine by me ;)

kevinresol commented 5 years ago

I am fine with the Lazy, but I prefer keeping ms

kevinresol commented 5 years ago

Let's merge, the failure in test is not related to this PR