GeoJSON support (writer at first)

[akidee]

I added a GeoJSONWriter class (without providing JS access), defined in src/geojsonwriter.hpp which can accessed in Geometry::toJSON(decimalPlaces, bbox):

• decimalPlaces (default -1) can be from -1 to 15
• bbox (default false) will add the bbox property
  Tests are added.

[booo]

Sorry for the late response. Thx for all your work. Some thoughts of mine:

• We should use the same coding style
• There should be a separate example for GeoJSON
• We should rename the function from toJSON to toGeoJSON
• Do we actually need a class GeoJSONWriter?

I would prefer if there is a GeoJSONWriter class that mimics the WKTWriter, e.g. provides setTrim and setRoundingPrecision functions. This way we don't have to recreate the Writer on every call. Maybe we should still provide a shortcut like toGeoJSON on the Geometry.

I would also prefere if we keep the c++ code base as small as possible. My first idea for the problem was to write a WKT to GeoJSON parser. Works fine but maybe is not that efficient if you want bboxes and SRIDs on your GeoJSON output. For the SRID and GeoJSON you have to call c++ functions again.

I'm trying to merge this anyway. I really appreciate your work!

Regards
Philipp

[akidee]

Some comments:

1. Isn't JSON always GeoJSON in this case? I think yes, JSON is just another way of serializing or parsing a GEOS geometry. So what about defining toGeoJSON and using toJSON as alias? Renaming the functions to GEOS' conventions is OK
2. WKT -> GeoJSON: I have done this in the past. Since I am working with large datasets and will use node_geos on a server, I want to avoid this.
3. I wanted to keep it simple and not necessarily add a separate JS accessible class for converting to/from JSON.

[booo]

Ok. I did some cosmetic changes and added an alias for the toJSON function that is toGeoJSON. I'm going to export the GeoJSONWriter to the user. Makes sense to me because there is a WKTWriter and there will be a WKBWriter. The to(Geo)JSON function will stay as a shortcut. I'm going to add a small example too.

Did you benchmark point 2) ? Have you considered using other libs like geoscript for your software? Not sure if calling c/c++ code is the best way to go. Maybe we should add more async calls to the lib? Anyway cool to see the binding in action :)

[akidee]

No, but it is logical that it's slower, isn't it? Not just concerning CPU cycles but memory consumption, too. It's the user's responsibility to fork processes to do heavy computing etc. All the data is in memory, isn't it? This is why async calls are not needed. I have not studied your code in detail, but it seems that you have included some EIO calls in the background. Which function did you make async?
I am contributing since I am going to use the native binding, and the computation itself is surely faster if it's native. Just to represent a geometry is not the use of node-geos but for doing computations.

Now I have remerged your changes into my repository ( like this: https://gist.github.com/964057 ).
Now git branch -a shows:

akidee/master

• master
  remotes/origin/HEAD -> origin/master
  remotes/origin/c++
  remotes/origin/master

Which branch do I need to checkout to continue development? I don't see the geojson branch which I find here. (Sorry for my lack of git knowledge, but this seems to be a little bit confusing ;) )

[akidee]

Oh, probably you should close the pull request since my latest commits, to synchronize your changes, are now included.

[booo]

These functions should be async:

isValid
isEmpty
isRectangle
disjoint
touches
intersects
crosses
within
contains
overlaps
equals
covers
coveredBy

I'm going to add a example for this too.

I'm not sure if asnc calls make sense in this context. Maybe your are right. Depends on the complexity of the functions and geometries.

Sorry for the confusion. I created a branch and plan to merge this back into the master soon.

[akidee]

No problem. Am I right that I can call them without a callback and they are sync functions then? If no, then I am not OK with this API. All these functions should be synchronous by default. In this context, it does not pay off.
Is it OK to close the pull request? It makes more sense for me to add other stuff to another PR.

[booo]

Add a callback and they become async functions. Without callback they are sync: 645f676#L0R14

Ok. I close this pull request and merge the geojson branch as soon as possible.