service() and single_service() return type other than object

[najdanovicivan]

@paulbalandan you've added strict object return type for service() and single_service() in 4.3.7

The issue I'm hading with this one is that I'm using services to return array of objects. So I had to override those functions in Common.php to remove the type hint

I don't see reason why service should only be an object and I thing the type hint should be set as mixed or at least to union type null|object|array but for union types PHP7 support needs to be dropped.

[kenjis]

In my understanding, a service is an object. Why do you think an array as a service?

[datamweb]

lonnieezell in codeigniter4 foundations Book:

Services are simple: it’s a way to create an instance of a class and all of its dependencies. It can either provide a brand new instance of the class, or a shared instance that will always be returned when that service is called. A shared instance is the default, as that allows simple class re-use throughout the framework.

Almost all of the system classes are available as a service. This allows you to easily grab, for example, the IncomingRequest instance that represents this request no matter where you are in the code.

And don’t worry, if you’re into testing, it provides a simple way to insert your own version for use during tests.

[kenjis]

I just realized, but is it necessary to return null? Why?

CodeIgniter4/system/Common.php

Line 1006 in 65c6a61

function service(string $name, ...$params): ?object

[paulbalandan]

It's because the phpdocs state that these functions return a shared instance (service()) or new instance (single_service) of a class. It says instance not instances so I believe that's why the names are appropriately singular. If you mean to return an array of services, then you should have instead made a services() function instead of overriding the existing ones and saying they are wrong to enforce return types. I believe your use case is a misuse.

[paulbalandan]

Null here means to say the requested service was not found.

[kenjis]

Oh, I got. Services returns null when not found. So service() also does. It is no problem.

[paulbalandan]

@lonnieezell what's your say on this?

[lonnieezell]

I don't see any issue with allowing people to return what they want, honestly. My original idea for it was to replace IoC containers in a simpler way, so it was originally envisioned serving only a single class. However, is there a reason to restrict end users from making creative uses out of it that we haven't thought about? I don't think so.

[kenjis]

If it is kind of a container, it may be better to be @return mixed.
https://github.com/php-fig/container/blob/707984727bd5b2b670e59559d3ed2500240cf875/src/ContainerInterface.php#L20-L22

[paulbalandan]

is there a reason to restrict end users from making creative uses out of it that we haven't thought about? I don't think so.

I think that's where the fault is. You make code with a vision in mind what it will do and what it will not. That is your contract with your end users. Allowing them to do virtually anything with it seems reckless, to say the least. It's like allowing me as your end user use the cache library to manipulate images and later complain why the cache driver is not saving the image. It's exaggerated but I hope I get my message through.

[lonnieezell]

You're exaggerating my thought a bit. The contract with the users originally was it would return a cached version of whatever they did within that method. And it was that way until we decided to make everything strictly typed. It was only then we changed the contract with the users.

We've all used it seen people use things in ways people didn't originally expect. And I'm sure sometimes we've been bit by it. But we are not talking about should we allow people to manipulate images with a cache library. We're talking about whether null values, something we currently allow to be returned, should allow to be cached, and getting hung up on their use case for it.

[kenjis]

It is clear that Services class was designed to return objects,

it was originally envisioned serving only a single class.

and all documentation tells it.

So aren't both Services that returns arrays and Services that returns null simply misused?
Is it really creative use?
Is null a service? Is an array a service? I don't think so.

If we change the Services to a container that can contain anything, it is better to do the following:

1. change the classname
2. throws an exception if the entry is not found

---

Services should be used as little as possible. Because:

1. It is an anti-pattern.
2. It is very slow.
   • CI4 performance regression #6889 (comment)
   • CI4 performance regression #6889 (comment)
   • fix: BaseService getSharedInstance should properly cache null #8611 (comment)

However, since Services is so easy to use, users may use them without limit.
So I don't think it is a good idea to extend the functionality of Services to make it more used.

The pattern of pulling dependent objects is an anti-pattern.
Client classes are tightly coupled with Services, but such tight coupling is not necessary if service instances are injected.

For example, this is not good. Foo is tightly coupled with Services.

class Foo
{
    public function bar()
    {
        $baz = service('baz'); // is equals to `Services::baz()`
        $baz->doSomething();
        // ...
    }
}

This is better. Foo works without Services:

class Foo
{
    public function __construct(private Baz $baz)
    {}

    public function bar()
    {
        $this->baz->doSomething();
        // ...
    }
}

[lonnieezell]

I'll say it again but to even think of completely changing that central of a component to something that works completely different is crazy and is a huge disservice to the users.

I will agree that looking at it now it has become more like a service locator than it originally was. If anything we should look at ways to simplify that process again. Maybe by adding a publish feature that copies the service methods from modules or composer packages into the app services class? That would allow us to get back to it just being a single static call without all of the other stuff we've added.

I think the argument about coupling is a little overblown in this case, honestly. You're already tightly coupled to the framework. It is relatively rare rarely that someone completely replaces the framework.

I think it's fine to encourage people to use the Services more responsibly by grabbing the service in the controller and passing it in as a dependency to child classes though that can get pretty ugly at times.

End of the day I feel like we have to come back and ask what is best for the users? That's not always what we learned in college. Sometimes simplifying and de-normalizing is necessary for ease of use and performance.

[kenjis]

I'm glad that you know the current Services is not so simple than it originally was.

I don't say that we should replace Services with something completely different soon.
Also, even if Services is replaced, it must be compatible with existing Services classes and not be a breakng change.

But I would like to say using Services is not so good.

If anything we should look at ways to simplify that process again. Maybe by adding a publish feature that copies the service methods from modules or composer packages into the app services class? That would allow us to get back to it just being a single static call without all of the other stuff we've added.

We could consider that approach, but it will be still a major change.
Also, moving a class method to another class is not easy, as it requires proper namespace handling, which is impossible without parsing the code.

I think it's fine to encourage people to use the Services more responsibly by grabbing the service in the controller and passing it in as a dependency to child classes though that can get pretty ugly at times.

We are already doing that. The following note has been around for quite some time.

Note
It is recommended to only create services within controllers. Other files, like models and libraries should have the dependencies either passed into the constructor or through a setter method.
https://codeigniter4.github.io/CodeIgniter4/concepts/services.html#why-use-services

[kenjis]

No member actively supports this proposal.
Close.