commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephen Colebourne <scolebou...@joda.org>
Subject Re: [Numbers] Inheritance and ValJO ?
Date Wed, 12 Dec 2018 23:47:25 GMT
I can see the paragraph I wrote way back when, but I'd disagree with myself
now. To be a VALJO you can't be abstract nor have subclasses, even a closed
set. I say this on the basis that AFAICT, future value types will also have
the same restrictions.

That said, VALJO rules are intended as a guide to best practice that may be
beneficial when value types arrive. I don't think the code you've got is
fundamentally wrong - its a reasonable way to share logic. An alternative
would be an enum `QuaternianType` that has methods.

final class Quaternion {
  private final QuaternionType type;
  private final double w;
private final double x;
private final double y;
private final double z;

 public double norm() {
   return type.norm(w, x, y, z);
 }

 public double normSq() {
   return type.norm(w, x, y, z);
 }
 // and so on
}

By delegating the methods via the type enum, you get flexible behaviour
without subclass.
Stephen




On Wed, 12 Dec 2018 at 23:20, Gilles <gilles@harfang.homelinux.org> wrote:

> Hi.
>
> On Wed, 12 Dec 2018 22:48:54 +0000, Stephen Colebourne wrote:
> > I think this has already been worked out, but the main reason for no
> > inheritance is that is probably blocks future conversion to value
> > types.
> > Composition instead of inheritance is usually the right solution.
>
> Thanks for the reply.
>
> Do you think that the implementation here:
>
>
> https://gitbox.apache.org/repos/asf?p=commons-numbers.git;a=blob;f=commons-numbers-quaternion/src/main/java/org/apache/commons/numbers/quaternion/Quaternion.java
> still counts as ValJO, despite allowing (mandating even) inheritance
> by inner classes (as per your paragraph that ends with "The need for
> this is rare however.")
>
> What I don't quite see is the consequences of the class not being
> final...
>
>
> Gilles
>
> >
> > Stephen
> >
> >
> > On Sun, 9 Dec 2018 at 10:21, Gilles <gilles@harfang.homelinux.org>
> > wrote:
> >
> >> Hello.
> >>
> >> After the discussion quote below, the conclusion was to go with
> >> inheritance:
> >>    https://issues.apache.org/jira/browse/NUMBERS-80
> >>
> >> However, it would make "Quaternion" fail the "ValJO" definition[1]
> >> that mandates that all constructors be private.
> >>
> >> Would a protected constructor really be an issue?
> >> [In the case of "Quaternion", the subclass constructor would only
> >> perform additional validation (cf. below for details).]
> >>
> >>
> >> Thanks,
> >> Gilles
> >>
> >> [1] https://blog.joda.org/2014/03/valjos-value-java-objects.html
> >>
> >> On Mon, 03 Dec 2018 10:31:42 +0100, Gilles wrote:
> >> > Hi.
> >> >
> >> > On Mon, 3 Dec 2018 03:56:02 +0000, Matt Juntunen wrote:
> >> >> I was just thinking from a practical standpoint. My current
> >> >> QuaternionRotation class is still in my working branch for
> >> >> GEOMETRY-14
> >> >> and so isn't really accessible to anyone. If I can finish it up
> >> in
> >> >> its
> >> >> current state (hopefully very soon) and get it merged, then
> >> someone
> >> >> else will be able to work with it and blend the functionality
> >> with
> >> >> commons-numbers.
> >> >
> >> > Someone else?
> >> >
> >> >>
> >> >> Here are some notes on your questions from before:
> >> >>
> >> >>   * Should "QuaternionRotation" inherit from "Quaternion"?
> >> >>
> >> >> That would work conceptually. The quaternions in the
> >> >> QuaternionRotation class are standard quaternions that meet two
> >> >> other
> >> >> criteria: 1) they are unit length, and 2) their scalar component
> >> is
> >> >> greater than or equal to zero (in order to standardize the angles
> >> >> involved).
> >> >
> >> > It seems indeed the perfect case for inheritance.
> >> >
> >> >> The one sticking point here is that I'm not sure how this
> >> >> fits with the VALJO concept. If we can get this sorted, then this
> >> >> very
> >> >> well may be the best option.
> >> >
> >> > What do you see as a potential issue?
> >> >
> >> >>
> >> >>   * Should "Quaternion" be defined in [Geometry] (and removed
> >> from
> >> >> [Numbers])?
> >> >>
> >> >> Perhaps. I've certainly only used them to represent 3D rotations.
> >> >> Are
> >> >> there any other use cases from commons-numbers?
> >> >
> >> > Not within [Numbers], but that's the point of those very low-level
> >> > components/modules: they are common utilities used by higher-level
> >> > components/libraries/applications.
> >> >
> >> > Given that "QuaternionRotation" is a special case of "Quaternion",
> >> > it is logical to keep the latter at a lower-level, namely in
> >> > [Numebers], and make [Geometry] depend on it.
> >> >
> >> >>
> >> >>   * Are some utilities defined in "QuaternionRotation" general
> >> >>     such that they could be part of the [Numbers] "Quaternion"
> >> API.
> >> >>     An example might be the transformation between quaternion and
> >> >>     matrix (represented as a double[3][3])?
> >> >>
> >> >> The conversion to rotation matrix and slerp are the best
> >> candidates
> >> >> here. The other methods rely on core classes from
> >> commons-geometry,
> >> >> namely Vector3D.
> >> >
> >> > Is "slerp" applicable to a general "Quaternion", or does it assume
> >> > the additional requirements of "QuaternionRotation"?
> >> > [Same question applies to all utilities in order to decide where
> >> to
> >> > define them.]
> >> >
> >> >>
> >> >> One more note: I decided to make a separate package for 3D
> >> rotations
> >> >> in my working branch for GEOMETRY-14, so QuaternionRotation is
> >> now
> >> >> at
> >> >>
> >> >>
> >>
> >>
> https://github.com/darkma773r/commons-geometry/blob/transforms/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/rotation/QuaternionRotation.java
> >> .
> >> >
> >> > Could you please update it so that it inherits from "Quaternion"?
> >> >
> >> > Thanks,
> >> > Gilles
> >> >
> >> >>
> >> >> -Matt
> >> >> ________________________________
> >> >> From: Gilles <gilles@harfang.homelinux.org>
> >> >> Sent: Sunday, December 2, 2018 3:57 PM
> >> >> To: dev@commons.apache.org
> >> >> Subject: Re: [Numbers][Geometry] Where to define "quaternion"
> >> (Was:
> >> >> Making Quaternion a VALJO)
> >> >>
> >> >> On Sun, 2 Dec 2018 19:20:03 +0000, Matt Juntunen wrote:
> >> >>> Unless anyone objects, I'm going to continue with what I'm
> >> working
> >> >>> on
> >> >>
> >> >> I certainly don't object on your working to improve the geometry
> >> >> code, but wherever that work overlaps with code being worked on
> >> >> elsewhere (in this case, the "Quaternion" class), then we'd
> >> >> rather have a discussion happening here first.
> >> >>
> >> >>> with QuaternionRotation and create a merge request. That way,
> >> we'll
> >> >>> at
> >> >>> least have a reference implementation and baseline functionality
> >> >>> for
> >> >>> commons-geometry that we can modify later based on what's
> >> decided
> >> >>> here.
> >> >>
> >> >> My questions below are a start; I'm waiting for answers.
> >> >> Code duplication is bad (TM).
> >> >>
> >> >> Regards,
> >> >> Gilles
> >> >>
> >> >>>
> >> >>> -Matt
> >> >>> ________________________________
> >> >>> From: Gilles <gilles@harfang.homelinux.org>
> >> >>> Sent: Saturday, December 1, 2018 9:40 PM
> >> >>> To: dev@commons.apache.org
> >> >>> Subject: Re: [Numbers][Geometry] Where to define "quaternion"
> >> (Was:
> >> >>> Making Quaternion a VALJO)
> >> >>>
> >> >>> On Sat, 01 Dec 2018 12:56:34 +0100, Gilles wrote:
> >> >>>> Hello.
> >> >>>>
> >> >>>> On Sat, 1 Dec 2018 06:05:31 +0000, Matt Juntunen wrote:
> >> >>>>> Hi guys,
> >> >>>>>
> >> >>>>> FYI, I've been working on a quaternion-related class named
> >> >>>>> QuaternionRotation for commons-geometry (see link below).
It
> >> >>>>> includes
> >> >>>>> slerp as well as several other geometry-oriented methods,
such
> >> as
> >> >>>>> conversion to/from axis-angle representations and creation
> >> from
> >> >>>>> basis
> >> >>>>> rotations. It's not quite ready for a merge yet since I
still
> >> >>>>> need
> >> >>>>> to
> >> >>>>> finish the Euler angle conversions.
> >> >>>>>
> >> >>>>> I did not use the Quaternion class from commons-numbers
since
> >> I
> >> >>>>> wanted to focus solely on using quaternions to represent
3D
> >> >>>>> rotations.
> >> >>>>> I felt like the commons-numbers class was too general for
> >> this.
> >> >>>>
> >> >>>> We need to explore further how to avoid duplication.
> >> >>>>
> >> >>>> Some questions:
> >> >>>>  * Should "QuaternionRotation" inherit from "Quaternion"?
> >> >>>>  * Should "Quaternion" be defined in [Geometry] (and removed
> >> from
> >> >>>>    [Numbers])?
> >> >>>>  * Are some utilities defined in "QuaternionRotation" general
> >> >>>>    such that they could be part of the [Numbers] "Quaternion"
> >> API.
> >> >>>>    An example might be the transformation between quaternion
> >> and
> >> >>>>    matrix (represented as a double[3][3])?
> >> >>>>
> >> >>>> The second consideration could apply to any computation that
> >> does
> >> >>>> not require types defined in [Geometry].  For example,
> >> >>>> interpolation
> >> >>>> is a purely quaternion-internal operation.
> >> >>>
> >> >>> s/second/third/
> >> >>>
> >> >>>>
> >> >>>> It looks to me that it should be possible to come up with a
> >> design
> >> >>>> that defines "rotation" in [Geometry] which uses a "quaternion"
> >> >>>> defined in [Numbers].
> >> >>>> Otherwise, one would wonder why "Complex" is also not in
> >> >>>> [Geometry]
> >> >>>> (for 2D rotations).
> >> >>>>
> >> >>>> Best regards,
> >> >>>> Gilles
> >> >>>>
> >> >>>>>
> >> >>>>> Regards,
> >> >>>>> Matt
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >>
> >>
> https://github.com/darkma773r/commons-geometry/blob/transforms/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/QuaternionRotation.java
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>> [https://avatars1.githubusercontent.com/u/3809623?s=400&v=4]<
> >>
> >>
> https://github.com/darkma773r/commons-geometry/blob/transforms/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/QuaternionRotation.java
> >> >
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>> darkma773r/commons-geometry<
> >>
> >>
> https://github.com/darkma773r/commons-geometry/blob/transforms/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/QuaternionRotation.java
> >> >
> >> >>>>> Apache Commons Geometry. Contribute to
> >> >>>>> darkma773r/commons-geometry
> >> >>>>> development by creating an account on GitHub.
> >> >>>>> github.com
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>> ________________________________
> >> >>>>> From: Gilles <gilles@harfang.homelinux.org>
> >> >>>>> Sent: Friday, November 30, 2018 9:37 AM
> >> >>>>> To: dev@commons.apache.org
> >> >>>>> Subject: Re: [numbers] Making Quaternion a VALJO
> >> >>>>>
> >> >>>>> On Fri, 30 Nov 2018 14:22:45 +0000, Steve Bosman wrote:
> >> >>>>>>> > and I have also emailed an ICLA.
> >> >>>>>>
> >> >>>>>>> Not received/acknowledged yet.
> >> >>>>>>
> >> >>>>>> I am now listed on the "Persons with signed CLAs but
who are
> >> not
> >> >>>>>> (yet)
> >> >>>>>> committers." page.
> >> >>>>>
> >> >>>>> Welcome!
> >> >>>>>
> >> >>>>>>> > I think two convenience divide methods performing
qr^{-1}
> >> and
> >> >>>>>>> r^{-1}q
> >> >>>>>>> > for q
> >> >>>>>>> > and r would be useful, but I couldn't think
of nice names
> >> for
> >> >>>>>>> them.
> >> >>>>>>
> >> >>>>>>> What are the use-cases?
> >> >>>>>>> Why aren't "multiply" and "inverse" enough?
> >> >>>>>>
> >> >>>>>> I must admit I'm new to quaternions and stumbled into
the
> >> >>>>>> project
> >> >>>>>> while
> >> >>>>>> trying to improve my understanding so I'm not going
to claim
> >> >>>>>> great
> >> >>>>>> knowledge of how common these operations are. I was
primarily
> >> >>>>>> thinking of
> >> >>>>>> Quaternion Interpolation - SLERP and SQUAD. It seems
to me
> >> that
> >> >>>>>> you
> >> >>>>>> end up
> >> >>>>>> creating inverse instances and throwing them away a
lot and I
> >> >>>>>> thought
> >> >>>>>> it
> >> >>>>>> would be good to reduce that overhead.
> >> >>>>>
> >> >>>>> Surely, the class "Quaternion" is minimal but, before adding
> >> to
> >> >>>>> the API, we be careful to have use-cases for low-level
> >> >>>>> operations.
> >> >>>>> Those mentioned above seems more high-level, tied to a
> >> specific
> >> >>>>> domain (see also "Commons Geometry", another new component
not
> >> >>>>> yet
> >> >>>>> released) but I may be wrong...
> >> >>>>>
> >> >>>>> Regards,
> >> >>>>> Gilles
> >> >>>>>
> >> >>>>>>
> >> >>>>>> Steve
> >> >>>>>
> >> >>>>
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message