docs icon indicating copy to clipboard operation
docs copied to clipboard

Incorrect argument in NutCommands

Open Boorj opened this issue 6 years ago • 6 comments

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.

Boorj avatar Jul 26 '18 08:07 Boorj

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

GwendolenLynch avatar Jul 26 '18 08:07 GwendolenLynch

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

Boorj avatar Jul 26 '18 16:07 Boorj

You should never use $app in a constructor.

GwendolenLynch avatar Jul 26 '18 16:07 GwendolenLynch

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

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

Boorj avatar Jul 26 '18 16:07 Boorj

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.

Boorj avatar Jul 26 '18 16:07 Boorj

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"

GwendolenLynch avatar Jul 27 '18 09:07 GwendolenLynch