Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isMultiPointInPoly variables oneInside is never used #2462

Open
qugemingzizhemefeijin opened this issue Jul 30, 2023 · 2 comments
Open

isMultiPointInPoly variables oneInside is never used #2462

qugemingzizhemefeijin opened this issue Jul 30, 2023 · 2 comments
Assignees

Comments

@qugemingzizhemefeijin
Copy link

qugemingzizhemefeijin commented Jul 30, 2023

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。

@JamesLMilner
Copy link
Collaborator

JamesLMilner commented Aug 20, 2023

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.

@smallsaucepan
Copy link
Member

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 smallsaucepan self-assigned this Dec 4, 2023
smallsaucepan added a commit to smallsaucepan/turf that referenced this issue Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants