search

mapbox / geobuf

A compact binary encoding for geographic data.
ISC License
967 stars 84 forks source link

another alternative fix for #90 (ensure property key strings are serialized in correct order) #93

Closed ivorblockley closed 6 years ago

ivorblockley commented 6 years ago

Another alternative to pick from. This is the "keep it simple approach".

ivorblockley commented 6 years ago

I suspect all three approaches will be similar in real-world performance, although it would be nice to have some speed benchmarks to test this. Otherwise the choice probably comes down to stylistic preference. #92 introduces an ES6 feature whereas this merge request along with #91 doesn't, which might also be a consideration.

mourner commented 6 years ago

92 is cleaner code-wise, but browser support is a concern — e.g. as far as I know iterating over the map.keys() with a for of loop isn't supported in IE and Safari versions <10. This version is as simple and does the job without upping the requirements, so I'll go with that. Looking at the code, performance shouldn't be affected. Thanks a lot for working on this!