search

codegram / futuroscope

Yet another Futures implementation in Ruby
MIT License
210 stars 13 forks source link

Passing through #nil? #15

Closed ggPeti closed 10 years ago

ggPeti commented 10 years ago

future { nil }.nil? returns false, I think this method should be forwarded.

Also recommend removing :clone from the forwarded methods, because duplication was fixed in #13, and I see no reason to return the encapsulated object when cloning. (That is not included in this PR.)

josepjaume commented 10 years ago

OMG you're so awesome! I shouldn't hurry so much releasing patchlevels, because the PR's keep coming :)

I'm not sure about the clone thing though, because it would probably encourage people to start using it and AFAIK it would be still broken when trying to clone a future that's not already resolved, right? Maybe we could tackle that specific issue, or explicitly raising a warning when using clone on an unresolved future. What do you think?

ggPeti commented 10 years ago

It's not broken, because cloning a Delegator will call its __getobj__, which is now protected by a mutex. So when you clone a future, it will block until the inner object is calculated, then clone that as well (standard Delegator behavior). Granted this is not ideal, we could defer blocking until the cloner actually uses the clone, but it's a good enough solution that doesn't require restructuring the whole library (perfect solution would be if the futures tracked their workers, not vice versa, but I like the current architecture more). Anyhow, I'll make the pull request with a spec demonstrating that clone works properly, feel free to decline it if you don't agree!

josepjaume commented 10 years ago

Oooh, I get it! Yes it looks like it should work. Thanks for your hard work, I'll gladly accept your PR :) On Jan 31, 2014 6:50 PM, "ggPeti" notifications@github.com wrote:

It's not broken, because cloning a Delegator will call its getobj, which is now protected by a mutex. So when you clone a future, it will block until the inner object is calculated, then clone that as well (standard Delegator behavior). Granted this is not ideal, we could defer blocking until the cloner actually uses the clone, but it's a good enough solution that doesn't require restructuring the whole library (perfect solution would be if the futures tracked their workers, not vice versa, but I like the current architecture more). Anyhow, I'll make the pull request with a spec demonstrating that clone works properly, feel free to decline it if you don't agree!

Reply to this email directly or view it on GitHubhttps://github.com/codegram/futuroscope/pull/15#issuecomment-33825192 .