[GitHub] [commons-geometry] singhbaljit commented on a change in pull request #122: Miscellaneous (cherry-picked) improvements

2020-12-22 Thread GitBox


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

2020-12-22 Thread GitBox


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

2020-12-22 Thread GitBox


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