Hi Matt, Another partial review here as I found a potential issue:
In core: core.partitioning.bsp.AbstractBSPTree The inner class 'AbstractNode<P extends Point<P>, N extends AbstractNode<P, N>>' has a depth property and a depth() method so is flagged by PMD. No real issue here but the depth() method does lazy loading of the depth if it is unknown (i.e. -1). However if the parent is null then this will repeat each time as the value is never computed and remains at -1. This applies to the root of the tree. In this case the depth method is documented as returning 0 but it will return -1. Any children will have their depth incorrect by 1. Is this a bug? I found elsewhere that the depth is set to 0 in the makeRoot() method. So perhaps this never manifests in your code. However since the class is public it can be overridden and perhaps used differently where the depth would be -1 for a root node. Alex --- Builds from the tag with: mvn clean verify *Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)* Maven home: /usr/local/apache-maven-3.6.3 Java version: 11.0.12, vendor: Eclipse Foundation, runtime: /Library/Java/JavaVirtualMachines/temurin-11.jdk/Contents/Home Default locale: en_GB, platform encoding: UTF-8 OS name: "mac os x", version: "11.5", arch: "x86_64", family: "mac" Builds from the source zip using: mvn clean verify site site:stage *Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)* Maven home: /usr/local/apache-maven-3.6.3 Java version: 1.8.0_301, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk1.8.0_301.jdk/Contents/Home/jre Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "10.16", arch: "x86_64", family: "mac" NOTICE.txt contains 2021. Test coverage is great at nearly 100%. There are a few lines that do not get executed in the spherical.twod package for some edge cases. No issues with Spotbugs. No major issues with PMD. Most of the issues should be easy to address, others can be ignored (god class, complexity rules). You have some public method getters returning boolean that start 'getXXX' rather than 'isXXX'. If this code style is consistent through the project then you can disable BooleanGetMethodName. It is found at least here: io/euclidean/threed/obj/AbstractObjPolygonReader.java io/euclidean/threed/obj/PolygonObjParser.java Releasing an API with a mix of getXXX and isXXX is not desirable. You have a few AvoidLiteralsInConditions when comparing double values to 0.0. You can get rid of these by using 0 in the condition or updating the rule to allow 0.0 as a magic number (untested but it should work). There are many private classes with private constructors or members that are used from outside the class. These can be made package private which should be more performant. This should use a locale for string conversion (e.g. Locale.ROOT): io/core/BoundaryIOManager.html#L484 Since this is used internally just for map keys it is not a blocker. User Guide: The equals() vs eq() section should highlight if -0.0 is equal to 0.0 (which is a recurrent issue with the IEEE float format depending on the context). The final code example should put a space after the '//' in this line: Point2S ptA = a.intersection(b); //(pi, pi/2)
