Skip to content

Conversation

@hannesvdvreken
Copy link
Contributor

Added methods resolvingType and afterResolvingType.

This would eliminate the need to execute a callable registered with afterResolving like here.

interface Baz {}
class Bar implements Baz {}
class Foo extends Bar {}

$container = new \Illuminate\Container\Container();

$container->afterResolvingType('Baz', function () {
    // This gets executed.
});

$container->make('Foo');

or in the example of the ValidationServiceProvider:

$this->app->afterResolvingAny('Illuminate\Contracts\Validation\ValidatesWhenResolved', function($resolved)
{
    $resolved->validate();
});

If you like this I can back this up with tests.

@GrahamCampbell
Copy link
Collaborator

Speaking of tests:

image

@JoostK
Copy link
Contributor

JoostK commented Dec 18, 2014

Seems nice, but all this logic and reflection adds way too much overhead for every object being created.

A different solution would be to iterate over all keys in the types array, such that you know exactly which types you need to be looking for. Avoids inspecting all objects (most of them won't have any listeners anyway) and accomplishes inherently the same as how it is currently implemented.

@hannesvdvreken
Copy link
Contributor Author

@JoostK Ah, wait. I could use the already created $object and just use the is_a method to filter. Brb, fixing implementation.

@GrahamCampbell yes, I've noticed. It's because of the Container::fireResolvingCallbacks method I guess. I'll fix that when there is an intention to accept this.

@hannesvdvreken
Copy link
Contributor Author

@JoostK better now?

@JoostK
Copy link
Contributor

JoostK commented Dec 18, 2014

Exactly! This is much better.

@JoostK
Copy link
Contributor

JoostK commented Dec 18, 2014

But probably just using a foreach and filling an array is much quicker:

protected function getCallbacksForType($object, array $types)
{
    $callbacks = array();

    foreach ($types as $type => $resolvers)
    {
        if ($object instanceof $type)
        {
            $callbacks = array_merge($callbacks, $resolvers);
        }
    }

    return $callbacks;
}

I'm only assuming this is quicker, not actually benchmarked it. There is much less array manipulation and function calls here though. Also, this has been written from the top of my head, not tested or anything.

@hannesvdvreken
Copy link
Contributor Author

@GrahamCampbell tests succeed, but I'll add some new ones to proof this is working as expected. Ok?

@hannesvdvreken
Copy link
Contributor Author

@JoostK I've been told array_ methods are more efficient because they can use C implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants