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

Incorrect argument in NutCommands #915

Open
Boorj opened this issue Jul 26, 2018 · 6 comments
Open

Incorrect argument in NutCommands #915

Boorj opened this issue Jul 26, 2018 · 6 comments

Comments

@Boorj
Copy link
Contributor

Boorj commented Jul 26, 2018

If you take a look here,
https://github.com/bolt/docs/blame/3.0/docs/extensions/intermediate/nut-commands.md#L39
you'll notice that argument is $container

protected function registerNutCommands(Container $container)
    {
        return [
            new Nut\DropBearCommand(),
            new Nut\KoalaCommand($container),
        ];
    }

If KoalaCommand extends Symfony\Component\Console\Command\Command it causes an error, because it expects $name as __constructor's argument.
Symfony/Component/Console/Command/Command.php#L69 :

public function __construct(string $name = null) {
   //...
}

If you extend Bolt\Nut\BaseCommand you can pass $container as argument and it works.

Nut/BaseCommand.php#L69 :

public function __construct(Container $app = null) {
   //...
}

But in this example KoalaCommand extends symfony's class.

class KoalaCommand extends Command { 
   //...
}

It deserves a comment in documents.

@GwendolenLynch
Copy link
Contributor

My mistake, the container shouldn't be passed into the constructor, rather the helper should be used.

@Boorj
Copy link
Contributor Author

Boorj commented Jul 26, 2018

yupp. Actually, the helper works as fallback and you still can pass $app to constructor.

@GwendolenLynch
Copy link
Contributor

You should never use $app in a constructor.

@Boorj
Copy link
Contributor Author

Boorj commented Jul 26, 2018

Niiiice. thanks, i'll try to do that. Usage in documents confuses a little bit

public function __construct(Container $app = null) {
   //...
}

@Boorj
Copy link
Contributor Author

Boorj commented Jul 26, 2018

i mean, i don't know why, but if you're telling that i'll try stick to your advice. Don't actually remember, am I using $app in constructors or not, but.. seems that 90% of my code inherits basic Bolt's classes (controllers, extensions, types). I suppose that won't be a problem to avoid.

@GwendolenLynch
Copy link
Contributor

Yeah, we've spent a while trying to remove it. As for the why, there is a tonne written, try Googling for "Service Locator anti-pattern"

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