Improve method names

CHANGELOG.md

@@ -0,0 +1,8 @@
+# Changelog
+
+## [x.x.x] xxxx-xx-xx
+- Coordinates set to 0, 0 will cause isGeocoded to return false
+
+## [1.0.0] 2024-08-05
+- First major version release
+

README.md

@@ -1 +1,6 @@
-Readme
+# PCM Geocode Bundle
+
+Provides an interface/trait combo to add latitude and longitude fields to an entity.
+
+Also included is a `Geocoder` service which accepts a postcode and attempts to return a `GeoCoordinates` object which contains the latitude and longitude of the postcode.
+

src/Entity/Interface/MappableInterface.php

@@ -1,18 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Pcm\GeocodeBundle\Entity\Interface;
-
-interface MappableInterface
-{
-    public function getLatitude(): ?float;
-
-    public function getLongitude(): ?float;
-
-    public function setLatitude(float $lat): self;
-
-    public function setLongitude(float $lon): self;
-
-    public function isGeocoded(): bool;
-}

src/Exception/ApiErrorException.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pcm\GeocodeBundle\Exception;
+
+class ApiErrorException extends GeocodeException
+{
+}
+

src/Exception/GeocodeException.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pcm\GeocodeBundle\Exception;
+
+class GeocodeException extends \RuntimeException
+{
+    public function __construct(string $postcode, string $message)
+    {
+        $formattedMessage = sprintf("Failed to geocode a postcode \"%s\": %s", $postcode, $message);
+
+        parent::__construct($formattedMessage);
+    }
+}
+

src/Exception/NoResultsFoundException.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pcm\GeocodeBundle\Exception;
+
+class NoResultsFoundException extends GeocodeException
+{
+    private const string MESSAGE = "No results found.";
+
+    public function __construct(string $postcode)
+    {
+        parent::__construct($postcode, self::MESSAGE);
+    }
+}
+

src/Interface/Entity/GeocodeInterface.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pcm\GeocodeBundle\Interface\Entity;
+
+/**
+ * Defines latitude and longitude getters and setters
+ */
+interface GeocodeInterface
+{
+    public function getLatitude(): ?float;
+
+    public function getLongitude(): ?float;
+
+    public function setLatitude(float $lat): self;
+
+    public function setLongitude(float $lon): self;
+
+    /**
+     * Used by {@see Pcm\GeocodeBundle\Service\Geocoder} to retrieve raw data for geocoding.
+     *
+     * @return string Data to use when geocodind an entity
+     */
+    public function getGeocodeData(): string;
+
+    /**
+     * @return bool True if both latitude and longitude are set and are not (0, 0)
+     */
+    public function isGeocoded(): bool;
+}
+

src/Service/Geocoder.php

@@ -4,10 +4,16 @@ declare(strict_types=1);
 
 namespace Pcm\GeocodeBundle\Service;
 
+use Pcm\GeocodeBundle\Exception\ApiErrorException;
+use Pcm\GeocodeBundle\Exception\NoResultsFoundException;
+use Pcm\GeocodeBundle\Interface\Entity\GeocodeInterface;
 use Pcm\GeocodeBundle\Model\GeoCoordinates;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 
+/**
+ * Find the geo-coordinates of a postcode
+ */
 final class Geocoder
 {
     private const string API_URL = "https://nominatim.openstreetmap.org/search";
@@ -15,24 +21,28 @@ final class Geocoder
     public function __construct(private HttpClientInterface $client) {}
 
     /**
-     * Convert a postcode into latitude and longitude. Returns null if conversion failed.
+     * Find and return the geo-coordinates of an entity
      *
-     * @param string $postcode
-     * @return null|GeoCoordinates
+     * @param GeocodeInterface $entity
+     *
+     * @return GeoCoordinates
+     *
+     * @throws NoResultsFoundException when no results were found for the provided postcode
+     * @throws ApiErrorException when the API response contains an error
      */
-    public function geocodePostcode(string $postcode): ?GeoCoordinates
+    public function geocode(GeocodeInterface $entity): GeoCoordinates
     {
+        $geocodeData = $entity->getGeocodeData();
         $client = $this->createClient();
-        $response = $this->makeApiRequest($client, $postcode);
+        $response = $this->makeApiRequest($client, $geocodeData);
         $data = $this->getDataFromResponse($response);
 
-        // @todo remove check, I don't think it's needed
-        if (empty($data)) {
-            return null;
+        if (array_key_exists('error', $data)) {
+            throw new ApiErrorException($geocodeData, $data['error']['message']);
         }
 
-        if (array_key_exists('error', $data)) {
-            return null;
+        if (empty($data)) {
+            throw new NoResultsFoundException($geocodeData, "No results found.");
         }
 
         return $this->createGeoCoordinates($data);
@@ -45,7 +55,7 @@ final class Geocoder
         ]);
     }
 
-    private function makeApiRequest(HttpClientInterface $client, string $postcode): ResponseInterface
+    private function makeApiRequest(HttpClientInterface $client, string $geocodeData): ResponseInterface
     {
         return $client->request(
             method: 'GET',
@@ -53,7 +63,7 @@ final class Geocoder
             options: [
                 'query' => [
                     'format' => 'json',
-                    'postalcode' => $postcode
+                    'q' => $geocodeData
                 ]
             ]
         );

src/Trait/Entity/GeocodeTrait.php

@@ -2,18 +2,16 @@
 
 declare(strict_types=1);
 
-namespace Pcm\GeocodeBundle\Entity\Trait;
+namespace Pcm\GeocodeBundle\Trait\Entity;
 
 use Doctrine\ORM\Mapping as ORM;
 
 /**
- * Allows an entity to be mapped via latitude and longitude coordinates
- *
- * Use MappableInterface
+ * Implementation of {@see Pcm\GeocodeBundle\Interface\Entity\GeocodeInterface}
  *
  * @package Pcm\GeocodeBundle
  */
-trait MappableTrait
+trait GeocodeTrait
 {
     #[ORM\Column(type: 'decimal', precision: 10, scale: 6, nullable: true)]
     private ?float $latitude = null;
@@ -45,13 +43,9 @@ trait MappableTrait
         return $this;
     }
 
-    /**
-     * Returns true if both latitude and longitude have been set
-     *
-     * @return bool
-     */
     public function isGeocoded(): bool
     {
-        return null !== $this->getLatitude() && null !== $this->getLongitude();
+        return null !== $this->getLatitude() && null !== $this->getLongitude() && (0.0 !== $this->getLatitude() && 0.0 !== $this->getLongitude());
     }
 }
+

tests/Service/GeocodeTest.php

@@ -1,68 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Pcm\GeocodeBundle\Tests;
-
-use Pcm\GeocodeBundle\Entity\Interface\MappableInterface;
-use Pcm\GeocodeBundle\Entity\Trait\MappableTrait;
-use Pcm\GeocodeBundle\Model\GeoCoordinates;
-use Pcm\GeocodeBundle\Service\Geocoder;
-use Pcm\GeocodeBundle\Tests\AppKernel;
-use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
-
-/**
- * We sleep 1 second after API calls to prevent breaking the API T&Cs.
- */
-final class GeocodeTest extends KernelTestCase
-{
-    // Buckingham Palace
-    private const string POSTCODE = 'SW1A 1AA';
-
-    private Geocoder $geocoder;
-
-    protected function setUp(): void
-    {
-        $kernel = new AppKernel('test', false);
-        $kernel->boot();
-        $this->geocoder = $kernel->getContainer()->get('pcm_geocode.geocoder');
-    }
-
-    public function testGeocodeInstance(): void
-    {
-        $this->assertInstanceOf(Geocoder::class, $this->geocoder);
-    }
-
-    public function testGeocodePostcodeReturnsNullOnInvalidInput(): void
-    {
-        sleep(1);
-        $this->assertNull($this->geocoder->geocodePostcode(''));
-    }
-
-    public function testGeocodePostcodeReturnsGeoCoordinates(): GeoCoordinates
-    {
-        sleep(1);
-        $geoCoords = $this->geocoder->geocodePostcode(self::POSTCODE);
-        $this->assertNotNull($geoCoords);
-
-        return $geoCoords;
-    }
-
-    /**
-     * @depends testGeocodePostcodeReturnsGeoCoordinates
-     */
-    public function testCoordinatesAreSet(GeoCoordinates $geoCoordinates): void
-    {
-        $this->assertIsFloat($geoCoordinates->latitude);
-        $this->assertIsFloat($geoCoordinates->longitude);
-    }
-
-    private function getMappableEntity(): MappableInterface
-    {
-        return new class implements MappableInterface
-        {
-            use MappableTrait;
-        };
-    }
-}
-

tests/Service/GeocoderTest.php

@@ -0,0 +1,96 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Pcm\GeocodeBundle\Tests;
+
+use Pcm\GeocodeBundle\Interface\Entity\GeocodeInterface;
+use Pcm\GeocodeBundle\Trait\Entity\GeocodeTrait;
+use Pcm\GeocodeBundle\Exception\ApiErrorException;
+use Pcm\GeocodeBundle\Exception\NoResultsFoundException;
+use Pcm\GeocodeBundle\Model\GeoCoordinates;
+use Pcm\GeocodeBundle\Service\Geocoder;
+use Pcm\GeocodeBundle\Tests\AppKernel;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+/**
+ * We sleep 1 second after API calls to prevent breaking the API T&Cs.
+ */
+final class GeocoderTest extends KernelTestCase
+{
+    // Buckingham Palace
+    private const string POSTCODE = 'SW1A 1AA';
+
+    private Geocoder $geocoder;
+
+    protected function setUp(): void
+    {
+        $kernel = new AppKernel('test', false);
+        $kernel->boot();
+        $this->geocoder = $kernel->getContainer()->get('pcm_geocode.geocoder');
+    }
+
+    public function testGeocodeThrowsOnEmptyInput(): void
+    {
+        sleep(1);
+        $this->expectException(ApiErrorException::class);
+        $entity = $this->createEntity('');
+        $this->expectExceptionMessageMatches("/Nothing to search for.$/");
+        $this->geocoder->geocode($entity);
+    }
+
+    public function testGeocodeThrowsOnInvalidPostcode(): void
+    {
+        sleep(1);
+        $this->expectException(NoResultsFoundException::class);
+        $this->expectExceptionMessageMatches("/No results found.$/");
+        $entity = $this->createEntity('Invalid Postcode');
+        $this->geocoder->geocode($entity);
+    }
+
+    public function testGeocodeReturnsGeoCoordinates(): GeoCoordinates
+    {
+        sleep(1);
+        $entity = $this->createEntity(self::POSTCODE);
+        $geoCoords = $this->geocoder->geocode($entity);
+        $this->assertNotNull($geoCoords);
+
+        return $geoCoords;
+    }
+
+    /**
+     * @depends testGeocodeReturnsGeoCoordinates
+     */
+    public function testCoordinatesAreSet(GeoCoordinates $geoCoordinates): void
+    {
+        $this->assertIsFloat($geoCoordinates->latitude);
+        $this->assertIsFloat($geoCoordinates->longitude);
+    }
+
+    private function getGeocodableEntity(): GeocodeInterface
+    {
+        return new class implements GeocodeInterface
+        {
+            use GeocodeTrait;
+        };
+    }
+
+    private function createEntity(string $data): GeocodeInterface
+    {
+        $entity = new class implements GeocodeInterface {
+            use GeocodeTrait;
+
+            public string $data;
+
+            public function getGeocodeData(): string
+            {
+                return $this->data;
+            }
+        };
+
+        $entity->data = $data;
+
+        return $entity;
+    }
+}
+

tests/Trait/GeocodeTraitTest.php

@@ -4,15 +4,15 @@ declare(strict_types=1);
 
 namespace Pcm\GeocodeBundle\Tests;
 
-use Pcm\GeocodeBundle\Entity\Interface\MappableInterface;
-use Pcm\GeocodeBundle\Entity\Trait\MappableTrait;
+use Pcm\GeocodeBundle\Interface\Entity\GeocodeInterface;
+use Pcm\GeocodeBundle\Trait\Entity\GeocodeTrait;
 use PHPUnit\Framework\TestCase;
 
-final class MappableTraitTest extends TestCase
+final class GeocodeTraitTest extends TestCase
 {
     private const float COORD = 123.456;
 
-    private MappableInterface $obj;
+    private GeocodeInterface $obj;
 
     protected function setUp(): void
     {
@@ -21,7 +21,7 @@ final class MappableTraitTest extends TestCase
 
     public function testSetLatitude(): void
     {
-        $this->assertInstanceOf(MappableInterface::class, $this->obj->setLatitude(self::COORD));
+        $this->assertInstanceOf(GeocodeInterface::class, $this->obj->setLatitude(self::COORD));
     }
 
     public function testGetLatitudeReturnsNull(): void
@@ -37,7 +37,7 @@ final class MappableTraitTest extends TestCase
 
     public function testSetLongitude(): void
     {
-        $this->assertInstanceOf(MappableInterface::class, $this->obj->setLongitude(self::COORD));
+        $this->assertInstanceOf(GeocodeInterface::class, $this->obj->setLongitude(self::COORD));
     }
 
     public function testGetLongitudeReturnsNull(): void
@@ -75,18 +75,30 @@ final class MappableTraitTest extends TestCase
         $this->assertTrue($this->obj->isGeocoded());
     }
 
-    public function testIsGeocodeReturnsTrueIfLatAndLonAreBothZero(): void
+    public function testIsGeocodedReturnsFalseIfLatAndLonAreBothZero(): void
     {
         $this->obj->setLatitude(0.000);
         $this->obj->setLongitude(0.000);
-        $this->assertTrue($this->obj->isGeocoded());
+        $this->assertFalse($this->obj->isGeocoded());
     }
 
-    private function getTraitObject(): MappableInterface
+    public function testIsGeocodedReturnsFalseIfLatAndLonAreBothZeroInts(): void
     {
-        return new class implements MappableInterface
+        $this->obj->setLatitude(0);
+        $this->obj->setLongitude(0);
+        $this->assertFalse($this->obj->isGeocoded());
+    }
+
+    private function getTraitObject(): GeocodeInterface
+    {
+        return new class implements GeocodeInterface
         {
-            use MappableTrait;
+            use GeocodeTrait;
+
+            public function getGeocodeData(): string
+            {
+                return '';
+            }
         };
     }
 }