[GitHub] [lucene] nknize commented on a diff in pull request #809: LUCENE-10514: Component2D#Within methods should return NOTWITHIN when the query geometry contains the triangle

[GitBox <gi...@apache.org>]

nknize commented on code in PR #809:
URL: https://github.com/apache/lucene/pull/809#discussion_r848570743


##########
lucene/core/src/java/org/apache/lucene/geo/Polygon2D.java:
##########
@@ -257,10 +257,13 @@ public WithinRelation withinLine(
       boolean ab,
       double bX,
       double bY) {
-    if (ab == true

Review Comment:
   I don't remember if order matters here? Seems before we would bail early if `ab == false` and skip the costly checks. Does that logic still stand if we move an `ab == false` check up front?



##########
lucene/core/src/java/org/apache/lucene/document/LatLonShapeBoundingBoxQuery.java:
##########
@@ -485,8 +485,11 @@ boolean containsTriangle(int aX, int aY, int bX, int bY, int cX, int cY) {
     }
 
     /** Returns the Within relation to the provided triangle */
-    Component2D.WithinRelation withinLine(int ax, int ay, boolean ab, int bx, int by) {
-      if (ab == true && edgeIntersectsBox(ax, ay, bx, by, minX, maxX, minY, maxY) == true) {
+    Component2D.WithinRelation withinLine(int aX, int aY, boolean ab, int bX, int bY) {

Review Comment:
   :+1:  I think this is cleaner



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org