Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should namespaces be used or fully written at method heads #309

Closed
sv3tli0 opened this issue Nov 16, 2016 · 3 comments
Closed

Should namespaces be used or fully written at method heads #309

sv3tli0 opened this issue Nov 16, 2016 · 3 comments

Comments

@sv3tli0
Copy link
Contributor

sv3tli0 commented Nov 16, 2016

I was working on something small and I decided to extend the Controller and its constructor.
https://github.com/bcit-ci/CodeIgniter4/blob/develop/system/Controller.php

How ever at the moment I got the same constructor name and all its param there came out an error:

Logger class which is defined as "use CodeIgniter\Log\Logger;" in my new Controller was not used and the system was looking for App/Controllers/Logger..

It isn't hard to add the same use definition into my class (and perhaps all its child's), but wouldn't it be better if all classes/interfaces which are inside any function head, are with their full namespace+class name instead of just the what is USE'd at the parent class ?

@lonnieezell
Copy link
Member

That's the fun of namespaces in general. But, if we were to force it on the user, they could never change it if they wanted to use a different one, like Monolog, etc. Granted, the recommended way is to override the logger method in the Services file, but that's a completely different story.

A simpler solution for your current problem, though, is something like this:

class Foo extends Controller
{
    public function __construct(...$params)
    {
        parent::__construct(...$params);

        // Do stuff here...
    }
}

That way you don't have to declare the exact class names in each controller when you need to perform actions within the constructor.

@sv3tli0
Copy link
Contributor Author

sv3tli0 commented Nov 16, 2016

I don't think that using . . . is the best solution.
The code is good at the moment..

use CodeIgniter\Log\Logger;
class Controller {
    public function __construct(
        RequestInterface $request,
        ResponseInterface $response, 
        Logger $logger = null
    ) { .. }

I personally don't have any problem (my PHP Storm is importing all "use" when I extend any class).
How ever if this is manually done it requires this Logger and those interfaces to match those declared at the original Controller.

My vision is of the logic behind the keyword "use" should be only then when a class is in fact used - called.
When it is only for declaration of some Type it's not the same.
Specially when those classes are extended further .

@lonnieezell
Copy link
Member

My vision is of the logic behind the keyword "use" should be only then when a class is in fact used - called.
When it is only for declaration of some Type it's not the same.

There's the crutch, though. It is being used to pass a class in as a dependency injection and assigned to a class property. It's just a part of the language that you have to use the use statement if you type out the class name. I can't do anything about that :)

If we hardcode the namespace within the class constructor, you kill all of the benefits of dependency injection, so that's not something I'm willing to do.

It's a great vision, I just don't think it works in the overall picture. But the array packing and unpacking (...) does work quite nicely in this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants