Bug in simplify FeatureCollection

[istarkov]

This line https://github.com/Turfjs/turf-simplify/blob/master/index.js#L60 You change features property of function param, and this change input object.

Is it by design?

[nevi-me]

Hi @istarkov, I worked on the code there. Only seeing this issue now, will have a look at it by the weekend and revert back

[nevi-me]

Can you send me a sample of what you're seeing, input and output :smile:

[istarkov]

Can't understand about input and output :-) The issue is that function change it's argument, and this is antipattern for public API. (You can change argument params for private calls for example for performance reason)

Bad

const fn = (someObj) => {
   someObj.hello = 'wow'; // BAD for public API
   return someObj;
}

Good

const fn = (someObj) => {
   return {
     ...someObj,
     hello: 'wow',
  };
}

PS: A lot of old APIs do the same things, but as modern programming move towards functional programming style, the best is to make public api methods pure.

[nevi-me]

Sorry, but I don't understand. Please use the actual code as an example. I've looked at what I've changed and what was there before, I can't understand your comment.

  if (feature.type === 'FeatureCollection') {
    feature.features = feature.features.map(function (f) {
      simplified = simplifyHelper(f, tolerance, highQuality);

      // we create simpleFeature here because it doesn't apply to GeometryCollection
      // so we can't create it at simplifyHelper()
      if (supportedTypes.indexOf(simplified.type) > -1) {
        return simpleFeature(simplified, f.properties);
      } else {
        return simplified;
      }
    });

In the line that you referred to, I am mapping the Features in the FeatureCollection, similar to if I was looping through the array to create a new one, then replacing the FeatureCollection with the array.