Dependency Injection code smells

Posted On January 20, 2022

The use of injection does not automatically ensure that our code is correctly structured. While it’s an excellent mechanism for breaking dependencies and writing decoupled code, improper usage can introduce chaos into our system. There are certain code smells that, when detected, should alert you to potential design flaws in your code.

Number of Injected Parameters

In many programming languages, the general rule is that only three parameters should be injected into a method. However, since PHP is not a compiled language and the program is always “built from scratch” for each request, in the PHP domain, the guideline suggests up to five parameters can be injected. If you are injecting more than five, there’s likely an issue with your class (I suspect it might be doing more than it should, thereby violating the Single Responsibility principle). A common reason for this is populating attributes in a class. Instead of injecting individual parameters for each attribute, have you considered using an object to manage these parameter values? Perhaps you could utilize the Builder pattern, which initializes the object and then executes the method call working with that data?

Below, I’ll show an example of how to replace multiple parameters injected into a class to populate attributes by using a request object. This code example is based on Laravel Request, illustrating how to address incoming parameter injection.

First, we need to design a class that will convert the expected request parameters into object attributes.

use Illuminate\Http\Request;

abstract class RequestObject
{
    public function __construct(Request $request, array $attribs = [])
    {
        $local_attributes = get_object_vars($this);

        foreach ($local_attributes as $key => $val) {

            //set by value from request
            $this->$key = $request->input($key, null);

            //add or overrwrite with value from additional attributes
            if (isset($attribs[$key])) {
                $this->$key = $attribs[$key];
            }
        }
    }
}

then we can create the specified class, which objects will be injected. it could be something like that:

class Order extends RequestObject
{
    public  $user_id,
            $sort,
            $offer_type,
            $type,
            $order_type,
            $amount,
            $status = false,
            $symbol,
            $base_asset,
            $quote_asset,
            $stop,
            $limit,
            $laziness,
            $market_source,
            $accounts,
            $rate,
            $watching_rate,
            $deferred_activation,
            $account_credentials_finder,
            $market_order;
}

and finally we can use it:

/** in Controller: */

class OrdersController extends Controller
{
    //...

    public function index(Request $request)
    {
        $input = new OrderRequest($request, [
            'user_id' => auth()->id()
        ]);

        $orders = $orders->get($input);

        return new OrdersResources($orders);
    }

    //...
}

/** and in your business logic class */

use App\Http\Requests\RequestObjects\Order as OrderRequest;

class OrdersService 
{
    //... 

    public function get(OrderRequest $request): Collection
    {
        // ... some logic and conditions checking  here ...

        if ($request->status) {
            //...do something
        }

        //...

        return $this->order
                    ->inStatus($request->status)
                    ->byOfferType($request->offer_type)
                    ->byOrderType($request->order_type)
                    ->get();
    }

    //...
}

Why it’s bettter to create new class instead of using the laravel $request object directly (which become from controller)?

The reason is that:

you shouldn’t pass things which belongs to Controller layer into layer which is responsible for your business logic and are unknow
as mentioned in point 1 – you will know the object attributes so even your IDE will help you to use it, and you will not be forced to jump into many other files to check what parameters came from frontend (in the request).

Repeating Params

If you notice that some params are present in almost all of your class functions then you have two possible ways:

a) It’s just a simple refactor to do and you should move it to the __construct method

b) you have poor design and maybe the params in your object of that class should be set by some “setter” functions, in the class which use it?

So instead of something like this:

class OrderService
{

    public function get(User $user, $order_id)    
    {
        //...
    }

    public function create(User $user, OrderRequest $request)
    {
        //...
    }

    public funciton changeState(User $user, $order_id, $new_state)
    {
        //...    
    }

}

it’s better to write something like that:

class OrderService
{
    private $user;

    public function __construct(User $user)
    {
        $this->user = $user;
    }

    public function get($order_id)    
    {
        //...
    }

    public function create(OrderRequest $request)
    {
        //...
    }

    public funciton changeState($order_id, $new_state)
    {
        //...    
    }

}

Share on Social Media