You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/22 03:02:31 UTC

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

darkma773r commented on a change in pull request #122:
URL: https://github.com/apache/commons-geometry/pull/122#discussion_r547040251



##########
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 extends AbstractConvexHyperplaneBoundedRegion<P, S>> R transformInternal(
             final Transform<P> transform, final R thisInstance, final Class<S> boundaryType,
-            final Function<List<S>, R> factory) {
+            final Function<? super List<S>, R> factory) {

Review comment:
       I see the reasoning behind this but I'm not sure if it's needed since tt makes an already crazy method signature even crazier. WDYT? This goes for the other `super` generic changes as well.

##########
File path: commons-geometry-core/src/test/java/org/apache/commons/geometry/core/GeometryTestUtils.java
##########
@@ -120,7 +121,7 @@ public static Object serializeAndRecover(final Object obj) {
             final ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray());
             final ObjectInputStream si = new ObjectInputStream(bis);
             return si.readObject();
-        } catch (final Exception e) {
+        } catch (final IOException | ClassNotFoundException e) {

Review comment:
       We actually don't use this method at all so we could just delete it.

##########
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<Vector3D> pts) {
+        public Builder addAll(final Iterable<? extends Vector3D> pts) {

Review comment:
       Is this worth it? The only subclass of `Vector3D` is `Vector3D.Unit`.




----------------------------------------------------------------
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:
users@infra.apache.org