Refactor providers (part 1)

This PR is based on @MattKetmo's work. His initial idea was a sort of
eye-opener to me, hence this PR.

I removed useless methods in the AbstractProvider. I believe we could
have a AbstractLocaleAwareProvider too. The LocaleAwareProvider
interface does not make much sense at this point, I'd drop it.

The ProviderBasedGeocoder should be an aggregator or proxy.

The codebase is getting more and more readable and clean.

Edit: I chose to keep the LocaleAwareProvider interface as it is a
pretty common use case.

Author: willdurand

src/Geocoder/Dumper/Gpx.php

@@ -10,7 +10,7 @@
 
 namespace Geocoder\Dumper;
 
-use Geocoder\ProviderBasedGeocoder;
+use Geocoder\Geocoder;
 use Geocoder\Model\Address;
 
 /**
@@ -35,7 +35,7 @@ public function dump(Address $address)
     xsi:schemaLocation="http://www.topografix.com/GPX/1/0 http://www.topografix.com/GPX/1/0/gpx.xsd">
 
 GPX
-        , ProviderBasedGeocoder::VERSION);
+        , Geocoder::VERSION);
 
         if ($address->getBounds()->isDefined()) {
             $bounds = $address->getBounds();

src/Geocoder/Geocoder.php

@@ -25,7 +25,7 @@ interface Geocoder
     /**
      * Geocodes a given value.
      *
-     * @param string $value A value to geocode.
+     * @param string $value
      *
      * @return Address[]
      */
@@ -34,10 +34,28 @@ public function geocode($value);
     /**
      * Reverses geocode given latitude and longitude values.
      *
-     * @param double $latitude  Latitude.
-     * @param double $longitude Longitude.
+     * @param double $latitude.
+     * @param double $longitude
      *
      * @return Address[]
      */
     public function reverse($latitude, $longitude);
+
+    /**
+     * Returns the maximum number of Address objects that can be
+     * returned by `geocode()` or `reverse()` methods.
+     *
+     * @return integer
+     */
+    public function getLimit();
+
+    /**
+     * Sets the maximum number of `Address` objects that can be
+     * returned by `geocode()` or `reverse()` methods.
+     *
+     * @param integer $limit
+     *
+     * @return Geocoder
+     */
+    public function limit($limit);
 }

src/Geocoder/Provider/AbstractProvider.php

@@ -10,8 +10,8 @@
 
 namespace Geocoder\Provider;
 
+use Geocoder\Geocoder;
 use Geocoder\Model\AddressFactory;
-use Geocoder\ProviderBasedGeocoder;
 use Ivory\HttpAdapter\HttpAdapterInterface;
 
 /**
@@ -22,63 +22,53 @@ abstract class AbstractProvider
     /**
      * @var HttpAdapterInterface
      */
-    protected $adapter;
+    private $adapter;
 
     /**
-     * @var string
+     * @var AddressFactory
      */
-    protected $locale;
+    private $factory;
 
     /**
      * @var integer
      */
-    protected $maxResults = ProviderBasedGeocoder::MAX_RESULTS;
+    private $limit = Provider::MAX_RESULTS;
 
     /**
-     * @var AddressFactory
+     * @param HttpAdapterInterface $adapter An HTTP adapter
      */
-    protected $factory;
-
-    /**
-     * @param HttpAdapterInterface $adapter An HTTP adapter.
-     * @param string               $locale  A locale (optional).
-     */
-    public function __construct(HttpAdapterInterface $adapter, $locale = null)
+    public function __construct(HttpAdapterInterface $adapter)
     {
-        $this->setLocale($locale);
-
         $this->adapter = $adapter;
         $this->factory = new AddressFactory();
     }
 
     /**
-     * Returns the HTTP adapter.
-     *
-     * @return HttpAdapterInterface
+     * {@inheritDoc}
      */
-    public function getAdapter()
+    public function limit($limit)
     {
-        return $this->adapter;
+        $this->limit = $limit;
+
+        return $this;
     }
 
     /**
      * {@inheritDoc}
      */
-    public function setMaxResults($maxResults)
+    public function getLimit()
     {
-        $this->maxResults = $maxResults;
-
-        return $this;
+        return $this->limit;
     }
 
     /**
-     * Returns the maximum of wished results.
+     * Returns the HTTP adapter.
      *
-     * @return integer
+     * @return HttpAdapterInterface
      */
-    public function getMaxResults()
+    public function getAdapter()
     {
-        return $this->maxResults;
+        return $this->adapter;
     }
 
     /**
@@ -88,10 +78,15 @@ public function getMaxResults()
      */
     protected function getDefaults()
     {
-        return array(
+        return [
             'latitude'     => null,
             'longitude'    => null,
-            'bounds'       => null,
+            'bounds'       => [
+                'south' => null,
+                'west'  => null,
+                'north' => null,
+                'east'  => null,
+            ],
             'streetNumber' => null,
             'streetName'   => null,
             'locality'     => null,
@@ -104,7 +99,7 @@ protected function getDefaults()
             'country'      => null,
             'countryCode'  => null,
             'timezone'     => null,
-        );
+        ];
     }
 
     /**
@@ -114,33 +109,25 @@ protected function getDefaults()
      */
     protected function getLocalhostDefaults()
     {
-        return array(
+        return [
             'locality' => 'localhost',
             'region'   => 'localhost',
             'county'   => 'localhost',
             'country'  => 'localhost',
-        );
-    }
-
-    /**
-     * @param array $results
-     *
-     * @return array
-     */
-    protected function fixEncoding(array $results)
-    {
-        return array_map(function ($value) {
-            return is_string($value) ? utf8_encode($value) : $value;
-        }, $results);
+        ];
     }
 
     /**
      * @param array $data An array of data.
      *
      * @return \Geocoder\Model\Address[]
      */
-    protected function returnResult(array $data = [])
+    protected function returnResults(array $data = [])
     {
+        if (0 < $this->getLimit()) {
+            $data = array_slice($data, 0, $this->getLimit());
+        }
+
         return $this->factory->createFromArray($data);
     }
 }

src/Geocoder/Provider/ArcGISOnline.php

@@ -40,16 +40,16 @@ class ArcGISOnline extends AbstractProvider implements Provider
     private $protocol;
 
     /**
-     * @param HttpAdapterInterface $adapter       An HTTP adapter.
-     * @param string               $sourceCountry Country biasing (optional).
-     * @param bool                 $useSsl        Whether to use an SSL connection (optional).
+     * @param HttpAdapterInterface $adapter       An HTTP adapter
+     * @param string               $sourceCountry Country biasing (optional)
+     * @param bool                 $useSsl        Whether to use an SSL connection (optional)
      */
     public function __construct(HttpAdapterInterface $adapter, $sourceCountry = null, $useSsl = false)
     {
         parent::__construct($adapter);
 
         $this->sourceCountry = $sourceCountry;
-        $this->protocol = $useSsl ? 'https' : 'http';
+        $this->protocol      = $useSsl ? 'https' : 'http';
     }
 
     /**
@@ -58,7 +58,7 @@ public function __construct(HttpAdapterInterface $adapter, $sourceCountry = null
     public function geocode($address)
     {
         if (filter_var($address, FILTER_VALIDATE_IP)) {
-            throw new UnsupportedOperation('The ArcGISOnline does not support IP addresses.');
+            throw new UnsupportedOperation('The ArcGISOnline provider does not support IP addresses, only street addresses.');
         }
 
         // Save a request if no valid address entered
@@ -67,16 +67,14 @@ public function geocode($address)
         }
 
         $query = sprintf(self::ENDPOINT_URL, $this->protocol, urlencode($address));
-
-        $json = $this->executeQuery($query);
+        $json  = $this->executeQuery($query);
 
         // no result
         if (empty($json->locations)) {
-            throw new NoResult(sprintf('No results found for query %s', $query));
+            throw new NoResult(sprintf('No results found for query "%s".', $query));
         }
 
-        $results = array();
-
+        $results = [];
         foreach ($json->locations as $location) {
             $data = $location->feature->attributes;
 
@@ -89,7 +87,7 @@ public function geocode($address)
             $county       = !empty($data->Subregion) ? $data->Subregion : null;
             $countryCode  = !empty($data->Country) ? $data->Country : null;
 
-            $results[] = array_merge($this->getDefaults(), array(
+            $results[] = array_merge($this->getDefaults(), [
                 'latitude'     => $coordinates['y'],
                 'longitude'    => $coordinates['x'],
                 'streetNumber' => $streetNumber,
@@ -99,10 +97,10 @@ public function geocode($address)
                 'region'       => $region,
                 'countryCode'  => $countryCode,
                 'county'       => $county,
-            ));
+            ]);
         }
 
-        return $results;
+        return $this->returnResults($results);
     }
 
     /**
@@ -111,11 +109,10 @@ public function geocode($address)
     public function reverse($latitude, $longitude)
     {
         $query = sprintf(self::REVERSE_ENDPOINT_URL, $this->protocol, $coordinates[1], $coordinates[0]);
-
-        $json = $this->executeQuery($query);
+        $json  = $this->executeQuery($query);
 
         if (property_exists($json, 'error')) {
-            throw new NoResult(sprintf('No results found for query %s', $query));
+            throw new NoResult(sprintf('No results found for query "%s".', $query));
         }
 
         $data = $json->address;
@@ -127,16 +124,18 @@ public function reverse($latitude, $longitude)
         $county       = !empty($data->Subregion) ? $data->Subregion : null;
         $countryCode  = !empty($data->CountryCode) ? $data->CountryCode : null;
 
-        return array(array_merge($this->getDefaults(), array(
-            'latitude'    => $coordinates[0],
-            'longitude'   => $coordinates[1],
-            'streetName'  => $streetName,
-            'locality'    => $city,
-            'postalCode'  => $zipcode,
-            'region'      => $region,
-            'countryCode' => $countryCode,
-            'county'      => $county,
-        )));
+        return $this->returnResults([
+            array_merge($this->getDefaults(), [
+                'latitude'    => $coordinates[0],
+                'longitude'   => $coordinates[1],
+                'streetName'  => $streetName,
+                'locality'    => $city,
+                'postalCode'  => $zipcode,
+                'region'      => $region,
+                'countryCode' => $countryCode,
+                'county'      => $county,
+            ])
+        ]);
     }
 
     /**
@@ -147,43 +146,29 @@ public function getName()
         return 'arcgis_online';
     }
 
-    /**
-     * @param string $query
-     *
-     * @return string Query with extra params
-     */
     private function buildQuery($query)
     {
         if (null !== $this->sourceCountry) {
             $query = sprintf('%s&sourceCountry=%s', $query, $this->sourceCountry);
         }
 
-        return sprintf('%s&maxLocations=%d&f=%s&outFields=*', $query, $this->getMaxResults(), 'json');
+        return sprintf('%s&maxLocations=%d&f=%s&outFields=*', $query, $this->getLimit(), 'json');
     }
 
-    /**
-     * Executes a query
-     *
-     * @param string $query
-     *
-     * @throws NoResult
-     *
-     * @return \stdClass json object representing the query result
-     */
     private function executeQuery($query)
     {
-        $query = $this->buildQuery($query);
+        $query   = $this->buildQuery($query);
         $content = (string) $this->getAdapter()->get($query)->getBody();
 
         if (empty($content)) {
-            throw new NoResult(sprintf('Could not execute query %s', $query));
+            throw new NoResult(sprintf('Could not execute query "%s".', $query));
         }
 
         $json = json_decode($content);
 
         // API error
         if (!isset($json)) {
-            throw new NoResult(sprintf('Could not execute query %s', $query));
+            throw new NoResult(sprintf('Could not execute query "%s".', $query));
         }
 
         return $json;