bolt / docs

Documentation for Bolt.
45 stars 194 forks source link

Incorrect argument in NutCommands #915

Open Boorj opened 6 years ago

Boorj commented 6 years ago

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 commented 6 years ago

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

Boorj commented 6 years ago

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

GwendolenLynch commented 6 years ago

You should never use $app in a constructor.

Boorj commented 6 years ago

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

public function __construct(Container $app = null) {
   //...
}
Boorj commented 6 years ago

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 commented 6 years ago

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"