Hi Gilles, The hierarchy would be wrong from a conceptual POV: A vector can be described by Cartesian coordinates, but it should be possible to introduce new coordinate systems (polar in 2D, spherical in 3D) ... This approach doesn't limit the coordinate system at all. We can still make implementations of Point<EuclideanXD> and Vector<EuclideanXD> based on other coordinate systems. I think it'll actually be easier in this structure, since the details of the system are explicitly defined in a single base class. For example, to create polar vectors and points, we would create a PolarCoordinate2D base class and PolarPoint2D and PolarVector2D subclasses. ...algorithms that use vectors would/should still work (transparently doing the appropriate conversions if necessary). This is a general issue with the current code, separate from the changes I'm proposing here. I'm not introducing a new issue.
I understand (and agree with) the performance goal, but let's be careful to not define an API that does not correspond to the underlying concepts. Agreed. One vote in favor of having these utility methods is that I used some of them while transitioning the other geometry classes for my proof-of-concept. For example, o.a.c.geometry.euclidean.threed.Plane uses the Vector3D.dotProduct(Cartesian3D, Cartesian3D) method to compute the origin offset directly from the origin point and plane normal. What will happen when we introduce Spherical3D(r, theta, phi) alongside Cartesian3D(x, y, z) ? They should be able to get along just fine. They would each have subclasses that perform point and vector operations using their respective coordinate systems. The only issue would be trying to mix them, which as I mentioned above, is an existing issue with the current code base. However, I think having the coordinate systems encapsulated in base classes is a good first step in solving this. If "Cartesian3D" _implements_ "Point3D" and "Vector3D", it would still work (i.e. refactor so that "Point3D" becomes an interface and does not assume that the coordinates are Cartesian). I'm not quite sure what you're picturing for the Point3D interface here. Even so, if Cartesian3D implemented both interfaces, the compiler wouldn't be any help in catching simple programming errors. Thanks, Matt ________________________________ From: Gilles <gil...@harfang.homelinux.org> Sent: Monday, April 23, 2018 4:32 AM To: dev@commons.apache.org Subject: Re: [geometry] Points and Vectors Proposal Hi. On Mon, 23 Apr 2018 05:36:09 +0000, Matt Juntunen wrote: > Hi all, > > I'd like to propose an update to the Euclidean Point/Vector classes > in the geometry project. We currently have a single CartesianXD class > per dimension (eg, Cartesian2D) that implements both the Point and > Vector interfaces. This is similar to the previous commons-math > version where we had VectorXD classes that were also both Points and > Vectors. The change to the current version was through discussion on > MATH-1284 (https://issues.apache.org/jira/browse/MATH-1284). My > proposal is to flip the current inheritance hierarchy so that the > CartesianXD classes become the base classes for separate PointXD and > VectorXD classes. The hierarchy would be wrong from a conceptual POV: A vector can be described by Cartesian coordinates, but it should be possible to introduce new coordinate systems (polar in 2D, spherical in 3D) and algorithms that use vectors would/should still work (transparently doing the appropriate conversions if necessary). > PointXD classes only implement the Point interface > and VectorXD classes only implement Vector. The Cartesian base > classes > contain the actual x, y, z coordinate values along with a few other > common methods (such as getSpace()). For performance and convenience, > we can create static methods in the VectorXD classes that accept the > Cartesian base class instances, so that users can perform common > vector operations using either type. For example, if you have a giant > list of Points, these static methods would allow you to compute dot > products without needing to convert the Point instances to Vectors > first. I understand (and agree with) the performance goal, but let's be careful to not define an API that does not correspond to the underlying concepts. What will happen when we introduce Spherical3D(r, theta, phi) alongside Cartesian3D(x, y, z) ? > I've partially implemented this in a branch so you can get a better > idea of what I'm picturing: > https://github.com/darkma773r/commons-geometry/tree/point-vector. > The > commons-geometry-core and commons-geometry-euclidean sub-modules > contain the changes. > > [https://avatars1.githubusercontent.com/u/3809623?s=400&v=4]<https://github.com/darkma773r/commons-geometry/tree/point-vector> > > > darkma773r/commons-geometry<https://github.com/darkma773r/commons-geometry/tree/point-vector> > commons-geometry - Apache Commons Geometry > github.com > > > > The main benefit I see from this approach is code clarity. The intent > of the code seems much clearer to me when the names of the types > exactly match what they represent mathematically. For example, one of > the constructors for the Plane class currently looks like this: > > public Plane(final Cartesian3D p, final Cartesian3D normal, final > double tolerance) > > With my proposed changes, it would look like this: > > public Plane(final Point3D p, final Vector3D normal, final double > tolerance) > > The code is easier to read and the compiler will also help prevent > algorithm errors. That API is better indeed. If "Cartesian3D" _implements_ "Point3D" and "Vector3D", it would still work (i.e. refactor so that "Point3D" becomes an interface and does not assume that the coordinates are Cartesian). Best regards, Gilles > > Thoughts? > > Regards, > Matt Juntunen --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
