Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

Gilles Sadowski-2
-1
Personally, I don't agree with the rationale.

Anyways, such a change must be proposed on the "dev" ML and, if
agreed upon, be related to a JIRA report.

Regards,
Gilles

Le sam. 4 juil. 2020 à 13:44, <[hidden email]> a écrit :

>
> This is an automated email from the ASF dual-hosted git repository.
>
> mattjuntunen pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-geometry.git
>
> commit 3c25e07d19e0d7c36073713c6a60d15adf4a5ecf
> Author: XenoAmess <[hidden email]>
> AuthorDate: Sat Jul 4 10:16:55 2020 +0800
>
>     change GeometryInternalError to GeometryInternalException to better reflect its actual type
> ---
>  .../{GeometryInternalError.java => GeometryInternalException.java}    | 4 ++--
>  ...metryInternalErrorTest.java => GeometryInternalExceptionTest.java} | 4 ++--
>  .../java/org/apache/commons/geometry/enclosing/WelzlEncloser.java     | 4 ++--
>  .../geometry/euclidean/threed/rotation/QuaternionRotation.java        | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalError.java b/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalException.java
> similarity index 93%
> rename from commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalError.java
> rename to commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalException.java
> index bc52f62..caf209e 100644
> --- a/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalError.java
> +++ b/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalException.java
> @@ -20,7 +20,7 @@ package org.apache.commons.geometry.core.internal;
>   * This class is not intended to be part of the public API and should
>   * never be seen by the user when algorithms are functioning correctly.
>   */
> -public class GeometryInternalError extends IllegalStateException {
> +public class GeometryInternalException extends IllegalStateException {
>
>      /** Error message used for exceptions of this type. */
>      private static final String ERROR_MSG = "An internal geometry error occurred. This most often indicates an " +
> @@ -32,7 +32,7 @@ public class GeometryInternalError extends IllegalStateException {
>
>      /** Simple constructor with a default error message.
>       */
> -    public GeometryInternalError() {
> +    public GeometryInternalException() {
>          super(ERROR_MSG);
>      }
>  }
> diff --git a/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalErrorTest.java b/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalExceptionTest.java
> similarity index 91%
> rename from commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalErrorTest.java
> rename to commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalExceptionTest.java
> index 87198e1..4780365 100644
> --- a/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalErrorTest.java
> +++ b/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalExceptionTest.java
> @@ -19,12 +19,12 @@ package org.apache.commons.geometry.core.internal;
>  import org.junit.Assert;
>  import org.junit.Test;
>
> -public class GeometryInternalErrorTest {
> +public class GeometryInternalExceptionTest {
>
>      @Test
>      public void testMessage() {
>          // act
> -        final GeometryInternalError err = new GeometryInternalError();
> +        final GeometryInternalException err = new GeometryInternalException();
>
>          // assert
>          final String msg = "An internal geometry error occurred. This most often indicates an " +
> diff --git a/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java b/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java
> index 9f9f772..d94a0aa 100755
> --- a/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java
> +++ b/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java
> @@ -20,7 +20,7 @@ import java.util.ArrayList;
>  import java.util.List;
>
>  import org.apache.commons.geometry.core.Point;
> -import org.apache.commons.geometry.core.internal.GeometryInternalError;
> +import org.apache.commons.geometry.core.internal.GeometryInternalException;
>  import org.apache.commons.geometry.core.precision.DoublePrecisionContext;
>
>  /** Class implementing Emo Welzl's algorithm to find the smallest enclosing ball in linear time.
> @@ -98,7 +98,7 @@ public class WelzlEncloser<P extends Point<P>> implements Encloser<P> {
>              ball = moveToFrontBall(extreme, extreme.size(), support);
>              if (precision.lt(ball.getRadius(), savedBall.getRadius())) {
>                  // this should never happen
> -                throw new GeometryInternalError();
> +                throw new GeometryInternalException();
>              }
>
>              // it was an interesting point, move it to the front
> diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> index 62e950a..65f34c1 100644
> --- a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> +++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> @@ -19,7 +19,7 @@ package org.apache.commons.geometry.euclidean.threed.rotation;
>  import java.util.Objects;
>  import java.util.function.DoubleFunction;
>
> -import org.apache.commons.geometry.core.internal.GeometryInternalError;
> +import org.apache.commons.geometry.core.internal.GeometryInternalException;
>  import org.apache.commons.geometry.euclidean.internal.Vectors;
>  import org.apache.commons.geometry.euclidean.threed.AffineTransformMatrix3D;
>  import org.apache.commons.geometry.euclidean.threed.Vector3D;
> @@ -382,7 +382,7 @@ public final class QuaternionRotation implements Rotation3D {
>          }
>
>          // all possibilities should have been covered above
> -        throw new GeometryInternalError();
> +        throw new GeometryInternalException();
>      }
>
>      /** Get a sequence of angles around the given Tait-Bryan axes that produce a rotation equivalent
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

Matt Juntunen
What are your objections to the name? The new name is more consistent with JDK conventions.

Also, this class is not part of the public API.

-Matt
________________________________
From: Gilles Sadowski <[hidden email]>
Sent: Sunday, July 5, 2020 4:06 AM
To: Commons Developers List <[hidden email]>
Subject: Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

-1
Personally, I don't agree with the rationale.

Anyways, such a change must be proposed on the "dev" ML and, if
agreed upon, be related to a JIRA report.

Regards,
Gilles

Le sam. 4 juil. 2020 à 13:44, <[hidden email]> a écrit :

>
> This is an automated email from the ASF dual-hosted git repository.
>
> mattjuntunen pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-geometry.git
>
> commit 3c25e07d19e0d7c36073713c6a60d15adf4a5ecf
> Author: XenoAmess <[hidden email]>
> AuthorDate: Sat Jul 4 10:16:55 2020 +0800
>
>     change GeometryInternalError to GeometryInternalException to better reflect its actual type
> ---
>  .../{GeometryInternalError.java => GeometryInternalException.java}    | 4 ++--
>  ...metryInternalErrorTest.java => GeometryInternalExceptionTest.java} | 4 ++--
>  .../java/org/apache/commons/geometry/enclosing/WelzlEncloser.java     | 4 ++--
>  .../geometry/euclidean/threed/rotation/QuaternionRotation.java        | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalError.java b/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalException.java
> similarity index 93%
> rename from commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalError.java
> rename to commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalException.java
> index bc52f62..caf209e 100644
> --- a/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalError.java
> +++ b/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/internal/GeometryInternalException.java
> @@ -20,7 +20,7 @@ package org.apache.commons.geometry.core.internal;
>   * This class is not intended to be part of the public API and should
>   * never be seen by the user when algorithms are functioning correctly.
>   */
> -public class GeometryInternalError extends IllegalStateException {
> +public class GeometryInternalException extends IllegalStateException {
>
>      /** Error message used for exceptions of this type. */
>      private static final String ERROR_MSG = "An internal geometry error occurred. This most often indicates an " +
> @@ -32,7 +32,7 @@ public class GeometryInternalError extends IllegalStateException {
>
>      /** Simple constructor with a default error message.
>       */
> -    public GeometryInternalError() {
> +    public GeometryInternalException() {
>          super(ERROR_MSG);
>      }
>  }
> diff --git a/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalErrorTest.java b/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalExceptionTest.java
> similarity index 91%
> rename from commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalErrorTest.java
> rename to commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalExceptionTest.java
> index 87198e1..4780365 100644
> --- a/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalErrorTest.java
> +++ b/commons-geometry-core/src/test/java/org/apache/commons/geometry/core/internal/GeometryInternalExceptionTest.java
> @@ -19,12 +19,12 @@ package org.apache.commons.geometry.core.internal;
>  import org.junit.Assert;
>  import org.junit.Test;
>
> -public class GeometryInternalErrorTest {
> +public class GeometryInternalExceptionTest {
>
>      @Test
>      public void testMessage() {
>          // act
> -        final GeometryInternalError err = new GeometryInternalError();
> +        final GeometryInternalException err = new GeometryInternalException();
>
>          // assert
>          final String msg = "An internal geometry error occurred. This most often indicates an " +
> diff --git a/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java b/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java
> index 9f9f772..d94a0aa 100755
> --- a/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java
> +++ b/commons-geometry-enclosing/src/main/java/org/apache/commons/geometry/enclosing/WelzlEncloser.java
> @@ -20,7 +20,7 @@ import java.util.ArrayList;
>  import java.util.List;
>
>  import org.apache.commons.geometry.core.Point;
> -import org.apache.commons.geometry.core.internal.GeometryInternalError;
> +import org.apache.commons.geometry.core.internal.GeometryInternalException;
>  import org.apache.commons.geometry.core.precision.DoublePrecisionContext;
>
>  /** Class implementing Emo Welzl's algorithm to find the smallest enclosing ball in linear time.
> @@ -98,7 +98,7 @@ public class WelzlEncloser<P extends Point<P>> implements Encloser<P> {
>              ball = moveToFrontBall(extreme, extreme.size(), support);
>              if (precision.lt(ball.getRadius(), savedBall.getRadius())) {
>                  // this should never happen
> -                throw new GeometryInternalError();
> +                throw new GeometryInternalException();
>              }
>
>              // it was an interesting point, move it to the front
> diff --git a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> index 62e950a..65f34c1 100644
> --- a/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> +++ b/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> @@ -19,7 +19,7 @@ package org.apache.commons.geometry.euclidean.threed.rotation;
>  import java.util.Objects;
>  import java.util.function.DoubleFunction;
>
> -import org.apache.commons.geometry.core.internal.GeometryInternalError;
> +import org.apache.commons.geometry.core.internal.GeometryInternalException;
>  import org.apache.commons.geometry.euclidean.internal.Vectors;
>  import org.apache.commons.geometry.euclidean.threed.AffineTransformMatrix3D;
>  import org.apache.commons.geometry.euclidean.threed.Vector3D;
> @@ -382,7 +382,7 @@ public final class QuaternionRotation implements Rotation3D {
>          }
>
>          // all possibilities should have been covered above
> -        throw new GeometryInternalError();
> +        throw new GeometryInternalException();
>      }
>
>      /** Get a sequence of angles around the given Tait-Bryan axes that produce a rotation equivalent
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

Gilles Sadowski-2
Hi.

Le dim. 5 juil. 2020 à 13:26, Matt Juntunen
<[hidden email]> a écrit :
>
> What are your objections to the name? The new name is more consistent with JDK conventions.

It is a programming error (as opposed to a usage error, which
subclasses of "RuntimeException" represent).  As such it is
more self-documenting (IMHO) to keep the previous name.
JDK's use of the English word "Error" should not prevent all
application to use it.  There is no implied relationship to the
JDK's "Error" hierarchy.

>
> Also, this class is not part of the public API.

An alternative to make it that clear is perhaps to have an
"InternalUtils" class (untested):

public class InternalUtils {
    private static String INTERNAL_ERROR_MESSAGE = "...";

    public static void throwInternalError() {
        throw new GeometryInternalError();
    }

    private static class GeometryInternalError extends IllegalStateException {
        // ...
    }
}

Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

Matt Juntunen
Hi Gilles,

I made the change because it seemed logical (to me) and sufficiently trivial (internal class with 2 usages). I don't really care too much either way so I'll go ahead and revert it. Commons Math has MathInternalError so there is precedent for this naming convention.

In regard to it not being part of the public API, this is indicated by its presence in the "o.a.c.geometry.core.internal" package.

-Matt





________________________________
From: Gilles Sadowski <[hidden email]>
Sent: Sunday, July 5, 2020 2:12 PM
To: Commons Developers List <[hidden email]>
Subject: Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

Hi.

Le dim. 5 juil. 2020 à 13:26, Matt Juntunen
<[hidden email]> a écrit :
>
> What are your objections to the name? The new name is more consistent with JDK conventions.

It is a programming error (as opposed to a usage error, which
subclasses of "RuntimeException" represent).  As such it is
more self-documenting (IMHO) to keep the previous name.
JDK's use of the English word "Error" should not prevent all
application to use it.  There is no implied relationship to the
JDK's "Error" hierarchy.

>
> Also, this class is not part of the public API.

An alternative to make it that clear is perhaps to have an
"InternalUtils" class (untested):

public class InternalUtils {
    private static String INTERNAL_ERROR_MESSAGE = "...";

    public static void throwInternalError() {
        throw new GeometryInternalError();
    }

    private static class GeometryInternalError extends IllegalStateException {
        // ...
    }
}

Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type

Gilles Sadowski-2
Hello.

Le mar. 7 juil. 2020 à 03:41, Matt Juntunen
<[hidden email]> a écrit :
>
> Hi Gilles,
>
> I made the change because it seemed logical (to me) and sufficiently trivial (internal class with 2 usages). I don't really care too much either way so I'll go ahead and revert it. Commons Math has MathInternalError so there is precedent for this naming convention.
>
> In regard to it not being part of the public API, this is indicated by its presence in the "o.a.c.geometry.core.internal" package.

Yes; we discussed that convention.
Using a method could perhaps avoid even declaring
a class. [It could reduce the number of errors in future
compatibility checks.]

Regards,
Gilles

>
> -Matt
>
>
>
>
>
> ________________________________
> From: Gilles Sadowski <[hidden email]>
> Sent: Sunday, July 5, 2020 2:12 PM
> To: Commons Developers List <[hidden email]>
> Subject: Re: [commons-geometry] 11/11: change GeometryInternalError to GeometryInternalException to better reflect its actual type
>
> Hi.
>
> Le dim. 5 juil. 2020 à 13:26, Matt Juntunen
> <[hidden email]> a écrit :
> >
> > What are your objections to the name? The new name is more consistent with JDK conventions.
>
> It is a programming error (as opposed to a usage error, which
> subclasses of "RuntimeException" represent).  As such it is
> more self-documenting (IMHO) to keep the previous name.
> JDK's use of the English word "Error" should not prevent all
> application to use it.  There is no implied relationship to the
> JDK's "Error" hierarchy.
>
> >
> > Also, this class is not part of the public API.
>
> An alternative to make it that clear is perhaps to have an
> "InternalUtils" class (untested):
>
> public class InternalUtils {
>     private static String INTERNAL_ERROR_MESSAGE = "...";
>
>     public static void throwInternalError() {
>         throw new GeometryInternalError();
>     }
>
>     private static class GeometryInternalError extends IllegalStateException {
>         // ...
>     }
> }
>
> Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]