Dependency Injection Code Smells and recurring parameters

Dependency Injection Code Smells and recurring parameters

The use of injection doesn’t guarante that our code is already done in the right way.
It’s a great mechanism for breaking dependencies and writing decoupled code. When used incorrectly, it can also bring chaos to our code.
There are few code smells which, if are noticed, should warn you that the code isn’t properly designed.

Amount of Injected Params

In most of the programming languages the rule says that only 3 params should be injected in the method.
However – because PHP is not a compiled language and the program is always “built from scratch” for each request,
In the PHP world, the rule says that there are 5 parameters that can be injected.
If you are injecting more than 5 – there is probably something wrong with your class. My guess is that it is doing more than it should and Single Responsibility is broken.
One of the most common reasons is filling the attributes in class. What you could do instead of injecting a single param for each attribute, you should think about using a special object. It will handle the values of those params.
Additionally, use some pattern Builder that inilializes the object. After that, It will make method calls that operate on that data.

Below you can find an example how to replace many params which are injected to class to fill attributes by using a request object.
This code example is based on Laravel Request and it presents how to resolve incoming params injection.

Firstly, we need to prepare a class which will transform the request params that we expect to have into the object attributes.

use illuminate\Http\Request;

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

        forech ($local_attributes as $key => $val) {
            //set by value from request
            $this->$key = $request->input($key, null);
            //add or overwrite with value from additional attributes
            if (isset($attribs[$key])) {
                $this->$key = $attribs[$key];
            }
        }
    }
}

use illuminate\Http\Request;

abstract class RequestObject

{

public function __construct(Request $request, array $attribs = []}

{

$local_attributes = get_object_vars($this)

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

//set by value from request

$this->$key = $request->input($key, null);

//add or overwrite with value from additional attributes

if (isset($attribs[$key])) {

$this->$key = $attribs[$key];

}

Secondly, we can create the specified class and which objects will be injected. As a result, 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;
}

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: */
xlass 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 xhecking here...
        if ($request->status) {
            //...do something
        }
        //...
        return $this->order
                    ->inStatus($request->status)
                    ->byOfferType($request->offer_type)
                    ->byOrderType($request->order_type)
                    ->get();
    }
    //...
}

/** in Controller: */

xlass 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 xhecking here...

if ($request->status) {

//...do something

}

//...

return $this->order

->inStatus($request->status)

->byOfferType($request->offer_type)

->byOrderType($request->order_type)

->get();

}

//...

}

Why is it better to create new class instead of using the Laravel $request object directly (which comes from controller)?

The reason is that:

You shouldn’t pass things which belong to Controllers layer into layer which is responsible for your business logic and are unknown.
As mentioned in point 1 – you will know the object’s attributes

Repeating Params

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

You just need to do a simple refactor and you should move it to the __construct method
You have poor design, so 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 that:

class OrderService
    public function get(User $user, $order_id)
    {
        //...
    }
    public function create(User $user, OrderRequest $request)
        //...
    }
    public function changeState(User $user, $order_id, $new_state)
    {
        //...
    }
}

class OrderService

public function get(User $user, $order_id)

{

//...

}

public function create(User $user, OrderRequest $request)

//...

}

public function changeState(User $user, $order_id, $new_state)

{

//...

}

it will be better to do 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 function changeState($order_id, $new_state)
    {
        //...
    }
}

Class OrderService

{

private $user

public function __construct(User $user)

{

$this->user = $user;

}

public function get($order_id)

{

//...

}

public function create(OrderRequest $request)

{

//...

}

public function changeState($order_id, $new_state)

{

//...

}

Subsequent articles

Static Factory Methods aka Named Constructors: Read more…

Refactoring Example – guard clauses: Read more…

Hard-coded parts: Read more…

Dependency Injection code smells: Read more…

Naming conventions: Read more…

Improving conditions clauses – few small things with hudge impact: Read more…

Clean Code – how to start: Read more…