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 <gil...@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 <gil...@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 <gil...@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 <gil...@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 <gil...@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-unsubscr...@commons.apache.org > >> For additional commands, e-mail: dev-h...@commons.apache.org > >> > >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >