Torann / laravel-geoip

Determine the geographical location of website visitors based on their IP addresses.
http://lyften.com/projects/laravel-geoip
BSD 2-Clause "Simplified" License
2.1k stars 373 forks source link

Allow dependency injection #203

Closed KaliaJS closed 3 years ago

KaliaJS commented 3 years ago

Allow dependency injection. To avoid using a global variable.

    use Torann\GeoIP\GeoIP;

    public function login(GeoIP $geoip)
    {
        return $geoip->getLocation($request->ip())->toArray();
    }
KaliaJS commented 3 years ago

@Torann What do you think ?

Torann commented 3 years ago

Nice but it breaks the helper. Also I don't really see the need, why not use the helper? geoip()

KaliaJS commented 3 years ago

@Torann I avoid global variables as much as possible. For me it's a bad practice (sometimes it's inevitable). I submit a new commit with the correction for the break and in addition I added a deferred to the service provider.

KaliaJS commented 3 years ago

@Torann ??

Torann commented 3 years ago

Sorry I just don't see a benefit to this. The helper is not a global variable, it's a function and this method is pretty common practice. I would recommend if you don't care for using it then to continue using your fork in your projects. Also in your fork you can completely remove the helper function and do as you please :-)

KaliaJS commented 3 years ago

@Torann No problem