13

Say I have an interface CrawlerInterface with implementation PageCrawler and FeedCrawler; if we happen to need both classes in a controller, how can that be achieved with constructor injection?

Previously we use a central ServiceProvider to register (i.e. App::bind) such classes, but in most cases we only have 1 implementation of an interface, so said problem hasn't occured to us yet.

PS: I also wonder if this problem suggests we should split the controller.


Updates:

Thanks for the comments and response, to explain, said interface has only one public method: crawl($uri), and both page/feed crawler implements it as given a resource identifier, return resource.


My follow up question:

Say we are in a calculator scenario where Addition, Subtraction and Multiplication share the same interface Operation, which has only 1 public method run, at some point we will still encounter this problem right? How do we handle situation like these in general with ServiceProvider?

bitinn
  • 9,188
  • 10
  • 38
  • 64
  • 1
    As some have pointed out, this may actually be a code smell (One that I've certainly run into myself!). Can you include *why* you chose to use an interface to implement? What differs between your 2 implementations? You may actually want to use concrete classes for dependencies as Antonio pointed out, or re-think the code architecture a little. Perhaps we can help with the underlying architecture which makes this an issue. – fideloper Oct 14 '13 at 21:21
  • @fideloper thx, think I have a solution to the problem; but I am still a bit confused about how far can we take such approach, see my follow up questions. – bitinn Oct 15 '13 at 03:33
  • 1
    I really think it's not a bad thing to have several implementations to an interface. However, if that's the case and you want to use many implementations at the same time, then there should be a reason based on their usage. For example I could have 2 caches in my application: one for the queries, the other one for my views. I then use "APCCache" for the queries and "FileCache" for the views, but as you can see, I don't care about the implementations, I just care that I have a "cache.queries" and "cache.views". It's the usage that's important. – Matthieu Napoli Oct 15 '13 at 07:56
  • @MatthieuNapoli i wonder how would one go about managing this with service provider... – bitinn Oct 17 '13 at 08:51
  • I don't know about Laravel but usually DI containers offer the concept of "named services", i.e. a service not identified by a class/interface name but by an arbitrary name. I would request from the container the "cache.queries" entry, not the "Doctrine\Cache\CacheInterface" entry. – Matthieu Napoli Oct 17 '13 at 08:53
  • @MatthieuNapoli if it's not too much to ask, could you update your answer with an example with controller, i can't seem to figure out how to test or mock such named services... – bitinn Oct 17 '13 at 09:07
  • @bitinn I've updated my answer with examples, and to be more clear of the difference between each solution. Let me know if it's clearer now. – Matthieu Napoli Oct 17 '13 at 09:16
  • @MatthieuNapoli thx it's much clearer, and i got mine working locally as well. i would like to wait a bit to see if there are better suggestion before giving out bounty :) – bitinn Oct 17 '13 at 09:41

3 Answers3

15

If each crawler exists for a different reason, you can use arbitrary names for your instances, for example:

App::bind('crawler.allArticles', 'PageCrawler');
App::bind('crawler.latestArticles', 'FeedCrawler');

For the controller:

App::bind('CrawlerController', function($app) {
    return new CrawlerController(
        App::make('crawler.allArticles'),
        App::make('crawler.latestArticles')
    );
});

Your controller code would then use each crawler differently:

public function showLatestArticlesAction()
    $latestArticles = $this->latestArticlesCrawler->crawl();
    // ...
}

public function showAllArticlesAction()
    $allArticles = $this->allArticlesCrawler->crawl();
    // ...
}

If you just have a list of crawlers where each is used for the same thing, you probably want to do something like:

App::bind('crawlers', function($app) {
    return [
        App::make('PageCrawler'),
        App::make('FeedCrawler'),
    ];
});

In your controller, you'll get a list of "crawlers" by configuring it like so:

App::bind('CrawlerController', function($app) {
    return new CrawlerController(App::make('crawlers'));
});

Your controller code could be something like this:

public function showArticlesAction()
    $allArticles = array();
    foreach ($this->crawlers as $crawler) {
        $allArticles = array_merge($allArticles, $this->crawler->crawl());
    }
    // ...
}
Matthieu Napoli
  • 48,448
  • 45
  • 173
  • 261
  • say i use `CrawlerInterface.page` - is this like having an alias to PageCrawler, does it have advantage over injecting classes directly? – bitinn Oct 15 '13 at 03:46
  • @bitinn in your example, yes it's the same and you shouldn't do that. The name should be about a different usage. For example in my app I have a "logger.main" (log errors) and a "logger.queries" (log DB queries). Then I choose which logger I want depending on what I want to log, not depending if I want to log to a file, to syslog, or to [FirePHP](http://www.firephp.org/). – Matthieu Napoli Oct 15 '13 at 07:51
  • So you should "name" your services when you have the same kind of object used for different things. However in your case, if you just want to get "all crawlers" to crawl all possible sources, then the 2nd solution (in my answer) is better. – Matthieu Napoli Oct 15 '13 at 07:52
  • seems like nobody came up with a better alternative, my first bounty is yours to take, thx for the help! – bitinn Oct 24 '13 at 04:32
6

Ok lets assume you have a CrawlerController

class CrawlerController extends BaseController 
{
    protected $crawler1;
    protected $crawler2;

    public function __construct(CrawlerInterface $c1, CrawlerInterface $c2)
    {
        $this->crawler1 = $c1;
        $this->crawler2 = $c2;
    }
}

an interface

interface CrawlerInterface{}

and concrete implementations of that intefrace called PageCrawler and FeedCrawler

class PageCrawler implements CrawlerInterface{}
class FeedCrawler implements CrawlerInterface{}

You would inject the dependencies by writing a service locator like

App::bind('CrawlerController', function($app) {
    $controller = new CrawlerController(
        new PageCrawler,
        new FeedCrawler
    );
    return $controller;
});

But as suggested by others you should rethink your logic, use it only if this kind of architecture is unavoidable

Nenad
  • 3,438
  • 3
  • 28
  • 36
  • Hmmm, I have never seen usa-case like this, should we really be returning the controller from `App::bind` closure? what implication does it have? – bitinn Oct 15 '13 at 03:42
  • Since I never use this approach, I cannot say for sure if there are any implications, but on the first sight, you provide the dependencies manually instead of laravel doing it via reflection so there shouldn't be any problems – Nenad Oct 15 '13 at 21:40
3

I think that the interface won't help you in this case.

By doing:

App::bind('CrawlerInterface', '<implementation>');

You need to choose one:

App::bind('CrawlerInterface', 'PageCrawler');

or

App::bind('CrawlerInterface', 'FeedCrawler');

And then Laravel will inject it:

class CrawlerController {

    public function __construct(CrawlerInterface $crawler)
    {
    }

}

To have both you have 2 options

-Have 2 different interfaces

-Inject the implementations directly:

class CrawlerController {

    public function __construct(PageCrawler $pageCrawler, FeedCrawler $feedCrawler)
    {
    }

}

But I also think that, if you need something like this, you better rethink your logic.

Antonio Carlos Ribeiro
  • 86,191
  • 22
  • 213
  • 204
  • what would have happen if we had `App::bind('CrawlerInterface', 'PageCrawler'); App::bind('CrawlerInterface', 'FeedCrawler'); ` in the service container register method and then the controller's constructor requires one input like this: `class CrawlerController { public function __construct(CrawlerInterface $crawler) { } } ` which implementation would be injected when `CrawlerController` is instantiated? – Ilesanmi John Jun 07 '22 at 18:06
  • Only the last one, that is FeedCrawler, would probably be injected in that case, since that's the last one that was bound. – Cat Jan 11 '23 at 18:07