turf icon indicating copy to clipboard operation
turf copied to clipboard

isMultiPointInPoly variables oneInside is never used

Open qugemingzizhemefeijin opened this issue 2 years ago • 2 comments
trafficstars

turf/packages/turf-boolean-within /index.ts

The following code:

function isMultiPointInPoly(multiPoint: MultiPoint, polygon: Polygon) {
  var output = true;
  var oneInside = false;
  var isInside = false;
  for (var i = 0; i < multiPoint.coordinates.length; i++) {
    isInside = booleanPointInPolygon(multiPoint.coordinates[i], polygon);
    if (!isInside) {
      output = false;
      break;
    }
    if (!oneInside) {
      // this variables Should be oneInside ???
      isInside = booleanPointInPolygon(multiPoint.coordinates[i], polygon, {
        ignoreBoundary: true,
      });
    }
  }
  return output && isInside;
}

this code variable oneInside Whether it is not used。

qugemingzizhemefeijin avatar Jul 30 '23 08:07 qugemingzizhemefeijin

Looking at this I agree that the code path seems slightly illogical in that !oneInside will always run as the variable is never reassigned. I'd need to probably look at the PR / gitblame / unit tests to understand the intent.

JamesLMilner avatar Aug 20 '23 11:08 JamesLMilner

Took a look at the git blame for this. The variable has been in there since day 1, and (though not conclusive) even Typescript reckons oneInside can only be false, meaning the code will always run:

Screenshot 2023-12-04 at 10 09 22 pm

I will remove the if() altogether and commit as a code prettification task in another PR.

smallsaucepan avatar Dec 04 '23 11:12 smallsaucepan