async-interop / event-loop

An event loop interface for interoperability in PHP.
MIT License
169 stars 9 forks source link

Add LoopDriverFactory #30

Closed kelunik closed 8 years ago

kelunik commented 8 years ago

The LoopDriverFactory SHOULD be used by driver implementations to set a default event loop driver.

This can be achieved by having a Composer autoloader like that:

{
    "autoload": {
        "files": ["src/bootstrap.php"]
    }
}

Where src/bootstrap.php contains the following code:

use Interop\Async\EventLoop\LoopDriverFactory;

class MyDriverFactory implements LoopDriverFactory
{
    public function create()
    {
        return new MyDriver();
    }
}

Loop::setFactory(new MyDriverFactory());

This ensures the user doesn't have to care about setting the actual driver. A user just has to require the specific event loop driver package he / she wants to use.

kelunik commented 8 years ago

It can be discussed whether LoopDriverFactory should be an interface or a simple callable, but I think it's fine having an interface.

trowski commented 8 years ago

This looks great. Libraries can set their own factories as the default, but users can always override if they wish.

bwoebi commented 8 years ago

I think using a simple callable is really good enough here; especially when an user wants to use his own factory, it ends up being more usable.

kelunik commented 8 years ago

Can we merge it or should I change it to be a simple callable if we anyway can't use return types?

AndrewCarterUK commented 8 years ago

I'd try and avoid discussion on anything more than implementation details on a PR.

If there's a significant design choice to be had, we should open an issue, summarise any arguments made so far and then start a discussion.

kelunik commented 8 years ago

@AndrewCarterUK That's an implementation detail.

AndrewCarterUK commented 8 years ago

Okay, I personally prefer the interface to a callable because it's more explicit (I am a factory that does this).

However, if other people disagree I'd prefer we discuss in an issue until we reach consensus.

kelunik commented 8 years ago

@bwoebi Could you please open an issue if you want to change it to a callable or respond that we can merge it?

bwoebi commented 8 years ago

@kelunik opened #42 to discuss this.

kelunik commented 8 years ago

Should we go with make or create?

bwoebi commented 8 years ago

CreateDriver is fine.

kelunik commented 8 years ago

You mean I should rename it from create to createDriver?

bwoebi commented 8 years ago

Oh, in factory. I thought you were talking about the static function on loop.

Nah, there just create is fine.

kelunik commented 8 years ago

I just applied the changes required for the registry (we also need a default one) and added the running bool, to reset loop / factory if a new factory is set and no loop is running.

I think this should be ready to merge then.

bwoebi commented 8 years ago

Looks good to me now ... I'd like to merge it soon, in case nobody objects until tomorrow.

trowski commented 8 years ago

LGTM. Let's rename it to Interop\Async\Loop\DriverFactory once #40 is merged.