artkonekt / concord

Laravel extension for building modular applications where modules are decoupled, re-usable and easily customizable
MIT License
206 stars 14 forks source link

Use Laravel IOC #2

Closed milosdakic closed 6 years ago

milosdakic commented 6 years ago

Love this concept of packages/modules/boxes. Something that use to be in Laravel but was taken out post 4.2.

Perhaps using standard Laravel IOC would be better long run?

$this->app->bind('Custom/Module/Model', Model::class);

Using it via the Laravel IOC

$model = app()->make('Custom/Module/Model');

This also then works for model binding without having to implement manual binding

Route::get('/{product}', function(Custom/Module/Model $product) {
  return $product;
});
fulopattila122 commented 6 years ago

Hello Milos,

thanks for the feedback! You're right about that, and it was designed to work with standard Laravel IOC.

The registerModel() method also silently binds the interface to the implementation with Laravel's service container so you can simply type hint the interface at any point where automatic injection happens.

This is described in depth in the Models Documentation.

This is the longest doc page, but actually contains all the whys and hows on this topic.

Actually the root of the problem is the model relationships.

The code is actually shorter: https://github.com/artkonekt/concord/blob/master/src/Concord.php#L124 ;)

milosdakic commented 6 years ago

public function user()
{
  return $this->hasOne(app('Custom/Module/Model'));
}

The above will work as you are resolving the dependency through the IOC, even if Custom/Module/Model is extended/overridden.

fulopattila122 commented 6 years ago

Yes, but the app('Custom/Module/Model') call makes a new surplus object every time.

milosdakic commented 6 years ago

Wouldn't the passed object be used if interacting with the relationship? Either way the relationship needs to make an instance of the object?

fulopattila122 commented 6 years ago

No, because eloquent creates new one(s) when fetches it/them from the db. Think about hasMany relations for example.

milosdakic commented 6 years ago

Or would the alternative to proxy's be to use Facades (as per the docs, it's exactly what you're trying to do)

https://laravel.com/docs/5.5/facades

fulopattila122 commented 6 years ago

Yes, I thought of that as well. The main difference is that facades are static resolvers to service objects (instances), but in this case we need a way to target classes. Plus you can't type hint facades. Also with facades all the time you'll have an object instance when you only need a string (actual class name)

Symfony projects achieve this with combining configuration and the service locator pattern. So if you need to know what is the class of 'product' you can return that from config 'product.class'.

You can do this with Laravel by using something like: return $this->hasOne(config('product.class));

spatie/permissions is using this approach for example.

I wanted to avoid coding in config files.

Concord's approach was loosely inspired from Laravel Spark that has a useModel() method for overriding default models.

fulopattila122 commented 6 years ago

Thanks for the questions, it helped me to refresh my thoughts about this part of Concord; I close this issue, but feel free to reopen if you have other questions/suggestion on this topic.