[GitHub] [commons-geometry] singhbaljit commented on a change in pull request #122: Miscellaneous (cherry-picked) improvements
singhbaljit commented on a change in pull request #122: URL: https://github.com/apache/commons-geometry/pull/122#discussion_r547330636 ## File path: commons-geometry-core/src/main/java/org/apache/commons/geometry/core/partitioning/AbstractConvexHyperplaneBoundedRegion.java ## @@ -161,7 +161,7 @@ public String toString() { */ protected > R transformInternal( final Transform transform, final R thisInstance, final Class boundaryType, -final Function, R> factory) { +final Function, R> factory) { Review comment: IMO, it improves the usability of the APIs. See Josh Bloch's _Effective Java_ (3rd Edition): > Using wildcard types in your APIs, while tricky, makes the APIs far more flexible. If you write a library that will be widely used, the proper use of wildcard types should be considered mandatory. Actually, some of the generic parameters can also use wildcards (i.e. `final Class boundaryType`), but I kept those out for now to avoid overcomplicating the signature even more. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-geometry] singhbaljit commented on a change in pull request #122: Miscellaneous (cherry-picked) improvements
singhbaljit commented on a change in pull request #122: URL: https://github.com/apache/commons-geometry/pull/122#discussion_r547331628 ## File path: commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/Bounds3D.java ## @@ -233,7 +233,7 @@ public Builder add(final Vector3D pt) { * @param pts points to add * @return this instance */ -public Builder addAll(final Iterable pts) { +public Builder addAll(final Iterable pts) { Review comment: Although there is one subclass within the library, users can define other subclasses of `Vector3D` (for whatever reason). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-geometry] singhbaljit commented on a change in pull request #122: Miscellaneous (cherry-picked) improvements
singhbaljit commented on a change in pull request #122: URL: https://github.com/apache/commons-geometry/pull/122#discussion_r547330636 ## File path: commons-geometry-core/src/main/java/org/apache/commons/geometry/core/partitioning/AbstractConvexHyperplaneBoundedRegion.java ## @@ -161,7 +161,7 @@ public String toString() { */ protected > R transformInternal( final Transform transform, final R thisInstance, final Class boundaryType, -final Function, R> factory) { +final Function, R> factory) { Review comment: IMO, it improves the usability of the APIs. See Josh Bloch's _Effective Java_ (3rd Edition): > Using wildcard types in your APIs, while tricky, makes the APIs far more flexible. If you write a library that will be widely used, the proper use of wildcard types should be considered mandatory. Actually, some of the generic parameters can also wildcards (i.e. `final Class boundaryType`), but I kept those out for now to avoid overcomplicating the signature even more. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org