Skip to content

Alias IoC key to Geocoder class for DI #41

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

Closed
wants to merge 1 commit into from
Closed

Alias IoC key to Geocoder class for DI #41

wants to merge 1 commit into from

Conversation

jszobody
Copy link

@jszobody jszobody commented Mar 4, 2016

In general I prefer injecting my dependencies when possible, instead of using facades.

Currently I can't inject Geocoder\Geocoder into my controller (or anywhere else) because Laravel would inject a new instance, not the one that your service provider wires up.

This pull request modified the service provider to alias geocoder to Geocoder\Geocoder and makes it DI friendly. This way I can:

function myControllerFunction(\Geocoder\Geocoder $geo) 
{
    // $geo is the pre-wired instance from the service provider!
}

Note I'm following the pattern from the AwsLaravel package:

https://github.com/aws/aws-sdk-php-laravel/blob/master/src/AwsServiceProvider.php#L53

@zanematthew
Copy link

When using this approach I receive an error

No provider registered.

What is the correct way to register providers when using DI?

public function __construct(\Geocoder\Geocoder $geocoder)
{

    var_dump($geocoder->geocode('some address'));

}

Note when using the facade as outlined in the readme, the geocode works as expected.

@mikebronner
Copy link
Member

This would not be the correct way to use it. The Facade actually contains a wired up instance with adapter and provider already set. Insttead of using DI here, perhaps using app('geocoder') would be a better route to go? Closing for now, but thatnks for putting the effort into the PR. :)

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

Successfully merging this pull request may close these issues.

3 participants