GeoJSONReader, minor changes to writer, suggested v0.3.0

[booo]

Could you do a git rebase? If you need any help let me know. This would help me merging this pull request. Thanks a lot for your awesome work!

@kashif do you have access to the npm repository? Could you maybe push a new version. Version update is in b19e848d74958939235844320c3a8e9abc36184d

[kashif]

yes I will push out a new npm version when you guys give me the go ahead

[kashif]

Ok merged into master. @booo could you kindly double check and then I will upgrade npm.

[akidee]

Have you found any potential memory leaks in my code? If so, please let me know.

[booo]

@akidee do you think there are potential memory leaks? If so could you provide use with a bit more background information.

I fixed the segfault/free issue and refined two test cases. Would be nice if you could review my changes. I'm going to merge the geojsonreader branch back into master as soon as I get an answer on the memory leak issue and know everything is fine for you.

Regards Philipp

[akidee]

To clarify my questions:

1) Tests: I usually use 3 kinds of asserts: equal, strictEqual, deepEqual. Any other comparisons can be included into this simple API, since I am just interested in the kind of equality. The reader of the code should not learn another API which is just doing the same thing. Adding a function isNull(value) is just value === null. And vows is pretty ugly for writing tests, because it wants to be nice. My test framework is just assert. But this is not what I want to discuss ;) 2) Memory management: Does a char* need to be freed that was created by using the quote notation? You can see that I throw simple error messages as simple strings. How and when do those strings - on the native side - get freed? I want to be sure not be suprised by any leaks in the future. 3) Permanent V8 strings: We could speed up the script by presaving the key names "coordinates" etc. as V8 strings (as static fields). I tried to use

Persistent<String> staticField; // header
staticField = Persistent<String>::New(String::New("coordinates")); // In ::Initialize()

The result was a segfault. I think this post clarifies that a symbol is needed: https://groups.google.com/forum/?fromgroups#!topic/v8-users/AXnMijKMIxs

Anything else looks fine so far.

[booo]

1) I'm not going to force you to use isNull() or whatever but most of vows aspects make sense to me so I use them. I think there current state is fine for everybody. The actual problem was the strictEqual here:

https://github.com/kashif/node-geos/commit/de81772117e2df3c06e9944092f5d01064aee994#L0L33

2) All strings you create with the quote notation are placed on the stack. As soon as your function returns they get removed from the stack. Maybe this is a good place for more information: http://www.learncpp.com/cpp-tutorial/79-the-stack-and-the-heap/ Is this what you asked for? I don't think they could be the cause of any memory leak. Maybe at other places in the code there are some hidden memory leaks. I'm not a c++ expert.

3) Will try this too.

[akidee]

Yes. But this was clear. I was just in doubt about returned or thrown values.

[booo]

Well I think there is no real difference.