3

I have recently come across the SOLID design principles and have been looking to find ways to incorporate dependency injection into my work so that I can write better tests and mock out any external calls. I have had a go at some dependency injection in PHP Laravel (Lumen) but something doesn't seem right in the below.

The code to create the dependencies is being created inside a public method in one of the controllers. This doesn't seem like the appropriate place to put it as I now have a lot of quite ugly code instantiating objects inside the controller rather than just receiving the request and packaging up the response. Is there somewhere more appropriate to the do the instantiation? Also for certain items, e.g. the $paying_user object. I am creating an empty Eloquent model, using a method to retrieve the first item in a collection and swapping it with the returned object to inject. This doesn't feel right either. Can anyone point me in what the best and proper way to structure the below should be?

public function cancel_subscription(Request $request)
{
    $result = false;

    // Create dependencies
    $platform_id = Sanitiser::clean_integer($request->input('platform_id'));
    $paying_user = new PayingUserModel();
    $paying_user = $paying_user->where('platform_id','=',$platform_id)->first();
    $current_subscription_model = $paying_user->non_cancelled_subscriptions->first();
    $refund_model = new RefundModel();
    $stripe = new Stripe();
    $stripe_refund = new Refund();
    $charge = new ChargeModel();
    $stripe::setAPIkey(env('STRIPE_SK'));
    $stripe_subscription = new Subscription();

    // Create services and inject
    $this->cancellation_service = new CancellationService($paying_user,$current_subscription_model,$stripe_subscription);
    $this->refund_service = new RefundService($paying_user,$current_subscription_model,$refund_model,$stripe_refund,$charge);

    // Run services
    $result = $this->cancellation_service->cancel_subscription();
    if($this->cancellation_service->get_refund_required()==true){
        $result = $this->refund_service->handle_refund();
    }
}
13
  • "the $paying_user object below it is swapped with an actual paying user object before being injected" ... what? Commented Dec 6, 2017 at 23:32
  • $paying_user = new PayingUserModel(); $paying_user = $paying_user->where('platform_id','=',$platform_id)->first() //trying to say that I'm creating an empty Eloquent Model, using it to retrieve the first item of a collection (to get the actual data) for the model I want to inject into my other classes. Commented Dec 6, 2017 at 23:33
  • the "... before being injected" part ... there is no 'injection' happening when you are calling methods or instantiating objects yourself Commented Dec 6, 2017 at 23:35
  • You are doing too much inside one method in Controller, at first you should separate the concerns in different type of objects/different places. Commented Dec 6, 2017 at 23:38
  • 1
    If I split the method and moved the refund activities to a new method, would the controller still be the most appropriate place to instantiate these empty objects and inject into the other objects? Commented Dec 6, 2017 at 23:42

1 Answer 1

2

If I understood well, you are trying to organize your code using dependency injection. I think the easiest way to do it, is by using Laravel's automatic injection.

Looking at your code, the class where cancel_subscription() is defined, depends of CancellationService and RefundService.

At the same time CancellationService depends of PayingUserModel and Subscription. And RefundService depends of PayingUserModel, RefundModel, Refund and ChargeModel.

You can use automatic injection by type hinting the arguments in each class constructor.

E.g. the CancellationService class constructor would look something like:

class CancellationService
{
    protected $payingUserModel;

    protected $subscription;

    public function __construct(PayingUserModel $payingUserModel, Subscription $subscription)
    {
        $this->payingUserModel = $payingUserModel;

        $this->subscription = $subscription;
    }
}

Now, for the PayingUserModel you want to pass a specific instance of the class.

This can be done by binding an instance to the container in the AppServiceProvider class.

$platform_id = request()->input('platform_id');

$user = PayingUserModel::where('platform_id', '=', $platform_id)->first();

$this->app->instance('PayingUserModel', $user);

Alternatively you could register the PayingUserModel instance as a singleton.

$this->app->singleton('PayingUserModel', function() use($platform_id) {
    return PayingUserModel::where('platform_id', '=', $platform_id)->first();
});

I didn't try it myself so I'm not 100% sure if this is the way to go. But according to the Laravel documentation this is how you manage dependency injection and hopefully it will give you an idea on how to proceed.

Sign up to request clarification or add additional context in comments.

2 Comments

Thanks Camilo, I've managed to implement something very close to the recommendation.
I'm glad I could help! Took me some time to understand the question and come up with a suggestion.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.