[jira] [Comment Edited] (GEOMETRY-17) Euclidean Vector Method Follow-Up

["Gilles (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GEOMETRY-17?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16622151#comment-16622151 ] 

Gilles edited comment on GEOMETRY-17 at 9/20/18 3:12 PM:
---------------------------------------------------------

{quote}we'd be double-computing the norm
{quote}
It should be avoided, indeed.

Could we perhaps use a similar "trick" to the one which you suggested for normalized vectors (i.e. a subclass with a {{norm}} field computed in the constructor)?

Then at place where validation is required, a new vector instance with a checked norm would be instantiated through a factory method:
{code:java}
public class Vector3D {
    // ...

    public Vector3D withCheckedNorm() {
        final double n = getNorm();
        if (isZeroNorm(n) ||         // To be implemented (?)
            Double.isInfinite(n) ||
            Double.isNaN(n)) {
            throw new IllegalNormException();
        }

        return new CheckedVector3D(this, n);
    }

    // ...

    private static class CheckedVector3D extends Vector3D {
        private final double norm;

        /**
         * @param orig Vector to copy.
         * @param norm Norm of {@code orig} vector.
         */
        CheckedVector3D(Vector3D orig, double norm) {
            super(orig.getX(), orig.getY(), orig.getZ());
            this.norm = norm;
        }

        @Override
        public double getNorm() {
            return norm;
        }

        @Override
        public Vector3D withCheckedNorm() {
            return this;
        }
    }
}
{code}
By the way,
 # class {{UnitVector}} does not override {{getNorm()}}, thus not taking advantage that the norm is known to be 1;
 # I wonder about having the other norms in the {{Vector}} API: removing them would make the subclasses leaner (otherwise we must keep and initialize one field per norm type, even though it seems that they'd be much less used, if at all). Within "Commons Geometry", {{getNorm1}} is only used in class {{o.a.c.geometry.euclidean.threed.enclosing.SphereGenerator}} and {{getNormInf}} is never used.


was (Author: erans):
{quote}we'd be double-computing the norm
{quote}
It should be avoided, indeed.

Could we perhaps use a similar "trick" to the one which you suggested for normalized vectors (i.e. a subclass with a {{norm}} field computed in the constructor)?

Then at place where validation is required, a new vector instance with a checked norm would be instantiated through a factory method:
{code:java}
public class Vector3D {
    // ...

    public Vector3D withCheckedNorm() {
        if (this instanceof CheckedVector3D) {
            return this;
        }

        final double n = getNorm();
        if (isZeroNorm(n) ||         // To be implemented (?)
            Double.isInfinite(n) ||
            Double.isNaN(n)) {
            throw new IllegalNormException();
        }

        return new CheckedVector3D(this, n);
    }

    // ...

    private static class CheckedVector3D extends Vector3D {
        private final double norm;

        /**
         * @param orig Vector to copy.
         * @param norm Norm of {@code orig} vector.
         */
        CheckedVector3D(Vector3D orig, double norm) {
            super(orig.getX(), orig.getY(), orig.getZ());
            this.norm = norm;
        }

        @Override
        public double getNorm() {
            return norm;
        }
    }
}
{code}
By the way,
 # class {{UnitVector}} does not override {{getNorm()}}, thus not taking advantage that the norm is known to be 1;
 # I wonder about having the other norms in the {{Vector}} API: removing them would make the subclasses leaner (otherwise we must keep and initialize one field per norm type, even though it seems that they'd be much less used, if at all). Within "Commons Geometry", {{getNorm1}} is only used in class {{o.a.c.geometry.euclidean.threed.enclosing.SphereGenerator}} and {{getNormInf}} is never used.

> Euclidean Vector Method Follow-Up
> ---------------------------------
>
>                 Key: GEOMETRY-17
>                 URL: https://issues.apache.org/jira/browse/GEOMETRY-17
>             Project: Apache Commons Geometry
>          Issue Type: Improvement
>            Reporter: Matt Juntunen
>            Priority: Major
>
> This is a follow-up issue to GEOMETRY-9. The following tasks should be completed:
>  # Vector2D - needs an orthogonal() method like Vector3D
>  # Vector#getMagnitude() should be removed. I originally added this as part of GEOMETRY-9 as an alias for getNorm(), but after thinking about it more and working with it, I believe it's more confusing than useful to have multiple names in the code base for the same idea.
>  # Vector#withMagnitude() should be renamed to Vector#withNorm() for the same reason as above.
>  # Vector#getRealNonZeroNorm() - This is currently a private method in the Vector implementation classes but I believe it is useful enough to be made public. The idea is that this would return the vector norm but throw an IllegalNormException if the norm is zero, NaN, or infinite. I've already come across some places in other classes (such as Rotation) where I want to use this.
>  
> Pull request: https://github.com/apache/commons-geometry/pull/11



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)