Making latitude and longitude coordinates nullable

[ActuallyConnor]

This proposed change will allow you to pass nullable arguments to the Coordinates class for the values of $latitude and $longitude.

Through using this library I have found myself every time creating a separate, custom Coordinates class that allows for nullable $latitude and $longitude values. There are many cases where you may wish to still instantiate a Coordinates object with one or both values being null, and then determining later on if the Coordinates class is usable by calling the hasLatitude() or hasLongitude() methods.

I am welcome to any and all feedback. This is after all a suggested change, and I won't take offence if it is not accepted.

Cheers! 😄

[ActuallyConnor]

@norkunas any chance you could review/run checks again, please?

[norkunas]

I'm not a maintainer of this repo, I maintain only geocoder-php/BazingaGeocoderBundle :)

From the logic pov I'm not sure - because without this PR getLatitude/getLongitude will return floats and doesn't throw, but after this PR it could throw, so I'd need to update code for these cases.

[ActuallyConnor]

Understood, I guess I will wait and see what is said then.

[jbelien]

Hello @ActuallyConnor ,

Thank you for your contribution.

I must admit I'm a bit confused by this request. Why would you create a location with only a longitude or only a latitude ? I can't really make sense of having coordinates with the longitude or latitude missing. The definition of coordinates is a longitude and a latitude.

There are many cases where you may wish to still instantiate a Coordinates object with one or both values being null, and then determining later on if the Coordinates class is usable by calling the hasLatitude() or hasLongitude() methods.

Could you give us some example of such use cases ?

[ActuallyConnor]

@jbelien so the use case we commonly have is reading and writing from nullable fields for latitude and longitude in the database. In the off chance that one is empty we would like the Coordinates object to still be created when reading from the database. And then when you wish to use the Coordinates you can check to see that they are set before accessing. Then on the access is will throw an exception if it is null. Hence the hasLatitude() and hasLongitude() methods.

Another use case is wanting to still return a Coordinates object in a function. See below for example.

public function geoCode(string $address, ?string $region = null) : Coordinates
{
    try {
        $coded = $this->geoCoder->geocodeQuery(GeocodeQuery::create($address)->withData('region', $region));
        $coordinates = $coded->first()->getCoordinates();

        return new Coordinates($coordinates->getLatitude(), $coordinates->getLongitude());
    } catch (Exception | GeocoderException $e) {
        return new Coordinates(null, null);
    }
}

In this case we created our own Coordinates class that allows for nullable properties. This ensure that the method itself does not return null.

If you don't see these use cases as good enough examples for as to why this PR would add value, then I understand. I just didn't like having to recreate the Coordinates class in order to meet these needs every time.

[norkunas]

In case geocoding cannot result coordinates, isn't it better to return null instead of a object with null values?

[jbelien]

In case geocoding cannot result coordinates, isn't it better to return null instead of a object with null values?

I fully agree with @norkunas !

public function geoCode(string $address, ?string $region = null) : Coordinates
{
    try {
        $coded = $this->geoCoder->geocodeQuery(GeocodeQuery::create($address)->withData('region', $region));
        return $coded->first()->getCoordinates();
    } catch (Exception | GeocoderException $e) {
        return  null;
    }
}

It makes no sense to have coordinates with "half" the parts. Coordinates must have both a longitude and a latitude, otherwise it's no coordinates.

What do you think @willdurand @Nyholm @atymic ?

[mtmail]

The try/catch seems a good enough, and my preferred, solution. I understand the specific usecase of nullable coordinates objects but I worry the average new user to the geocoder library would be too confused.

[ActuallyConnor]

I understand, no worries then. Thanks for at least taking the time to consider the PR 👍