async-interop / event-loop

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

Driver registry #68

Closed trowski closed 8 years ago

trowski commented 8 years ago

This PR supersedes #64 and addresses the issue raised in https://github.com/async-interop/event-loop/pull/65#discussion_r64792576.

Instead of being used by Loop, Registry should now be used by classes implementing Driver so each driver object contains state storage. While this is a violation of SRP, I believe this is the best compromise for associating data with a particular loop.

kelunik commented 8 years ago

Should we instead go with an abstract class then instead of an interface?

bwoebi commented 8 years ago

An abstract class is obviously a possibility, but I think here composition still wins over inheritance.

AndrewCarterUK commented 8 years ago

What is the argument against RegistryInterface (or just Registry) and LoopDriver::getRegistry()?

bwoebi commented 8 years ago

What is the advantage of this extra layer of access? In reality it'll just be Loop::getRegistry()->store(); and Loop::getRegistry()->fetch(); instead of Loop::storeState(); and Loop::fetchState(); which are more expressive and shorter.

(And don't argument with SRP here, because it's still a tiny violation as it still provides access to a registry, which is strictly seen a second responsibility)

Also, a trait is implementation wise much easier, it's like one single line and fine.

Thus I see no point in it.

sagebind commented 8 years ago

Too bad we don't have generics :cry:. A better solution would be:

interface RegistryState
{
    public static function init(): Self;
}

final class Registry
{
    private array $states;
    public function fetchState<T: RegistryState>(Driver $forDriver): T
    {
        $hash = someHashingFunction(typeName<T>(), $forDriver);
        if (!isset($this->states[$hash]) {
            $this->states[$hash] = T::init();
        }
        return $this->states[$hash];
    }
}

final class Loop
{
    private static Registry $registry;
    public static function fetchState<T: RegistryState>(): T
    {
        return static::$registry::fetchState<T>(static::get());
    }
}

class MyDnsResolver implements RegistryState { /* ... */ }

// Somewhere in code...
Loop::fetchState<MyDnsResolver>()->resolve("google.com");

But alas. We could still do this with ::class and typename strings, but it might not be as elegant.