Laravel Service Container: Am I doing the right thing?

Question

I have an invokable action class called: AttachMembersToRoom . Now, aside from basic form request validations, I also have complex validation, like comparing values across members, identify overlapping member rooms, and more... so I decided to make each validations into custom invokable classes, like CompareMembersValues, IdentifyOverlaps , and more....

At first, I did the following to execute every validations in AttachMembersToRoom :


class AttachMembersToRoom
{
    public function __invoke(MyData $data)
    {
        some logic here...

        foreach ($this->validators() as $validator) {
            $validator()($data);
        }
    }

    /**
    * @return callable[]
    **/
    private function validators(): array
    {
        return [
            fn () => app(CompareMembersValues::class),
            fn () => app(IdentifyOverlaps::class),
            ...
            ...
        ];
    }
}

but looking at it, I think it's a bit excessive to use app() for every validators I have.

so I decided to bind-tag it in the service container, like so:

$this->app->tag([
    CompareMembersValues::class,
    IdentifyOverlaps::class,
], 'memberValidators');

and in AttachMembersToRoom , I made the following adjustments:

class AttachMembersToRoom
{
    public function __construct(#[Tag('memberValidators')] protected iterable $memberValidators) {}

    public function __invoke(MyData $data)
    {
        some logic here...

        foreach ($this->memberValidators as $validator) {
            $validator($data);
        }
    }
}

At this point, I'm not even sure if I'm doing the right thing, or am I even overcomplicating things?

But my goal for all of this is the testability.. at some point, I want to mock each of these validators.

IMHO you're not overcomplicating here but rather following solid SOLID principles (pun intended). I'd stick to 2nd approach. — Marcin Orlowski
– Marcin Orlowski, Commented Nov 16 at 5:33

hakre · Accepted Answer · 2025-11-16 21:59:35Z

At this point, I'm not even sure if I'm doing the right thing, or am I even overcomplicating things?

That depends.

In your original design, both the array of class constants and their app-factory calls were hard-encoded inside, so an implementation detail with locality of behaviour and lazy object creation.

The node pattern of the invokable action class AttachMembersToRoom was well aligned with the node pattern of the Validator, which then by PHP's runtime guarantees could benefit from always exiting early by never returning, assumable via throws.

Given the name, AttachMembersToRoom, this appears to me as a good, well balanced, and smooth mixture.

What has changed then up to this point is that the array of class constants is injected into the apps' container state and their app() invocations moved before the AttachMembersToRoom::__construct() invocation. The ctor's parameter type is too wide thought and the parameter is not read-only.

Additionally the tagging requires to match the Tag attribute parameter that is a string operand which degrades the parameter as stringly-typed. That may sound ugly, but the only downside of that detail is, that at the tag point the use of the array of class-strings/objects is not visible. As you only have a single use-point and IIUC that will stay that way, this can likely be ignored.

The consequences of those changes are more prominent insofar that now all memberValidators are created before the AttachMembersToRoom is invoked.

I suggest you to trust your nose. You smell some over-doing here, and then my educated guess would be that you sensed the loss of the symmetry and balance of/between (all) invokable action classes and the node pattern. Perhaps we can even say: on two dimensions (invocable actions, invocable validators).

So this could have been the discovery then: The sequence of validators should be better represented as one validator function, so that a sequence of validators can be easily replaced with one (private) validator function so that such an validator could become an easy to inject invokable action class. Much neater design, smaller steps forward, less growing pain:

class AttachMembersToRoom
{
    public function __construct(
        #[InvokableValidatorActionClassSet()]
        private readonly array $validators = [
                                                 CompareMembersValues::class,
                                                 IdentifyOverlaps::class,
                                             ],
    )
    {
        // intentionally left blank //
    }

    public function __invoke(MyData $data)
    {
        // some logic here... //

        $this->invokeValidators($data);
    }

    private function invokeValidators(MyData $data)
    {
        foreach ($this->validators as $invokableValidatorActionClass)
        {
            $invokableValidator = app($invokableValidatorActionClass);
            $invokableValidator($data);
        }
    }
}

This defers any tagging by just using an attribute which suffices for static analysis, the default value is the configuration value, place of attribution is place of configuration (defaults), the parameter itself is configurable (inversion of control, e.g. unit testing), the class remains easy to create (important for using incl. testing).

We further begin to see the node pattern both for invokable actions and (invokable) validators.

The example also does not hide its incompleteness, app() is still hard encoded inside the foreach and not yet configurable. This is by intention, because when the validator is made an injectable invokable action class it remains the factory function, and with an injectable invokable action object, it would automatically move out of AttachMembersToRoom.

Until then, it is a single call point, functions are static and bindings can be changed by unfolding via use function statements.

So we can safely defer that until settlement.

First of all, thanks @hakre for the very detailed explanation. Your points on the discovery complexity of these validators and having a loose-type of the tag indeed adds some degrading factor. Will consider your suggested approach. But one thing regarding your remark regarding : The consequences of those changes are more prominent insofar that now all memberValidators are created before the AttachMembersToRoom is invoked.. Laravel handles it by the way by implementing the DefferableProvider interface: laravel.com/docs/12.x/providers#deferred-providers
@ssrsvn: Yes, thanks for pointing that deferable provider interface out, I wrote "invoked" and implied "created" because before __invoke() is avaiable __construct() must have been called. This will trigger resolution and henceforth the deferrable provider "to provide". The short version is perhaps: You loose lazy loading while deferrable providing still applies. E.g. all validators are instantiated vs. All until the first failing validator (including) are instantiated. And thanks for your comment, please see all this as discussion incl. the code example an trust your own nose. Exchange is good

Collectives™ on Stack Overflow

Laravel Service Container: Am I doing the right thing?

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related