Skip to content
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

Introducing a general support for native JSON serialization. #194

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bucefal91
Copy link

Here's a rather simple patch that allows spatial data to be json-serialized through native PHP toolset. I am far from being 100% sure this patch covers all use cases, but it does mine (Symfony FOS rest bundle with a JSON content type).

@bucefal91 bucefal91 mentioned this pull request Aug 19, 2018
@coveralls
Copy link

coveralls commented Aug 19, 2018

Coverage Status

Coverage decreased (-17.9%) to 80.119% when pulling 7013441 on bucefal91:140-json-serializable into 58ea5fa on creof:master.

Alexandre-T added a commit to Alexandre-T/doctrine2-spatial that referenced this pull request Mar 28, 2020
@Alexandre-T
Copy link

This request #209 which implements JsonSerializable. You could have a look at this fork. Deployed on packagist, a new package is now available for PHP7.2+

*/
public function jsonSerialize()
{
return $this->toArray();
Copy link

@tugrul tugrul Jun 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also type should be in the json data to make compatible to GeoJSON spec. Better implementation:

public function jsonSerialize()
{
    return ['type' => $this->getType(), 'coordinates' => $this->toArray()];
}

@@ -65,6 +65,14 @@ public function toJson()
return json_encode($json);
Copy link

@tugrul tugrul Jun 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can replace to:

public function toJson()
{
    return json_encode($this);
}

Copy link

@tugrul tugrul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments about GeoJSON compatibility

@Alexandre-T Alexandre-T mentioned this pull request Jul 6, 2020
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.

4 participants