Bug 1415 : modify PVector to include better methods for chaining operations
Last modified: 2009-12-24 08:56




Status:
ASSIGNED
Resolution:
-
Priority:
P1
Severity:
enhancement

 

Reporter:
davbol
Assigned To:
fry

Attachment Type Created Size Actions

Description:   Opened: 2009-12-23 08:49
As of 1.0.9
Now that PVector has become sort of "mainstream", in that for many uses it's no
longer necessary for an end-user to go to an external library for vector/matrix
support, it'd be nice to see it fleshed out a bit more for *end-user* use.

For example, an obvious ommision is a PVector(PVector source) constructor. Which
currently requires "PVector a = new PVector(); a.set(b);" Similarly, the presence of
set(float []), that implies it'd also be useful to have a PVector(float[] source)
constructor.

The other nicety involves the issue of static methods that return new instances,
versus methods that operate on the original. This setup works fine for discrete
single operations, but becomes cumbersome when stacking operations:

PVector a = new PVector();
PVector b = new PVector(1,2,3);
// c = a + b
PVector c = PVector.add(a,b); // not bad when used like this, but...
println("c = " + c);
// d = (a + (b * 4)) * c
PVector d = PVector.mult(PVector.add(a,PVector.mult(b,4)),c); // getting ugly
println("d = " + d);

Granted, there are alternative ways of wording that code - it's point is just to illustrate
the "wordiness" of creating new instances through stacked operations.

I think I do understand the intent of building it that way, for internal use by core,
reducing memory thrashing of temporary instances, but the end-user might prefer an
alternative syntax.

If the PVector class had a method perhaps named "addNew(PVector v)" (et al) that
created/operated on a new instance, you could instead write:

PVector d = (a.addNew(b).multNew(4)).multNew(c);

That approach is similar to Toxi's vector library (though opposite, where add()
implies a new instance, and addSelf() implies operating internally).

The Java Commons Math Vector3D also defines add() to return a new intance, but
then ALL of their methods return new - there are no "operate internally" methods -
don't really care for that approach either.

And it's not for me to argue about the naming conventions of opSelf vs opNew,
either is fine, but since the existing methods already operate internally, it'd
be "easier" to go with (something like) addNew() and leave add() doing what it
already does.

It's also not easy to just extend PVector as an end-user to implement such ideas
outside of core. It requires a bunch of ugly (and costly) "mangling" code just to gain
this minimal syntactic sugar, f.e.:

class PVectorX extends PVector {
// need to cover some constructors...
PVectorX() { super(); }
PVectorX(float x, float y, float z) { super(x,y,z); }
// this constructor is just to handle "type upgrade" in functions below
// (have to re-wrap the PVector results of static PVector methods into PVectorX's)
PVectorX(PVector v) { this.x=v.x; this.y=v.x; this.z=v.z; }
// then new stuff:
PVectorX addNew(PVector v) { return new PVectorX(PVector.add(this,v)); }
PVectorX multNew(float s) { return new PVectorX(PVector.mult(this,s)); }
PVectorX multNew(PVector v) { return new PVectorX(PVector.mult(this,v)); }
// etc...
}

PVectorX ax = new PVectorX();
PVectorX bx = new PVectorX(1,2,3);
PVectorX cx = ax.addNew(b);
println("cx = " + cx);
PVectorX dx = (ax.addNew(bx).multNew(4)).multNew(cx);
println("dx = " + dx);

(that compiles and runs, btw, but it ain't pretty!)

So it'd be nice to see all variations of input parameters available against all
variations of new/self/staticnew calling conventions on all methods (as applicable).

And, has been mentioned on the forum by others, fe:
http://processing.org/discourse/yabb2/YaBB.pl?num=1259250607
PVector could be made available as argument to many other methods. But that's
almost deserving of a separate topic and its own enh req.

Anyway, just some thoughts/ideas to put on a back burner somewhere.
Cheers
Additional Comment #1 From fry 2009-12-24 06:38
Thanks for the note. Do you have an idea of what the full list of methods
would look like? You've clearly given this some thought and I like the
direction.

For a copy of a vector, use get(), just like the image methods. I think
copy() also works.

You're correct re: the static methods, but I'm not really comfortable with
the general level of consistency in how they're implemented. In the end, it
may be too clever to overload add() in so many ways, and that a different
addStatic() method should be used instead for the efficient versions just
to keep them all straight.

For the other methods, I think we have another bug report for that (or it
was appended to another like here), but the issue is that it affects an
enormous number of methods (you'd be surprised to see the list), so I've
been avoiding expanding the function count that much.
Additional Comment #2 From davbol 2009-12-24 08:37
I hesitate to offer too much design suggestions, other than to just bring up issue for
better minds to ponder, because I'd been using Java Commons Math for so long that
I'm still trying to unlearn its way of doing things.

For example, with JCM even normalize() returns new. So you (often) find yourself
writing self-referential assignements like: a=a.normalize();
Because writing just a.normalize(); does nothing, result evaporates into the ether!

I think that approach might be even more confusing to p5's core audience. which is
why i largely agree that the default ops (that is, with the simple name, add() fe)
should be the operate internally versions (as is already the case).

So, to be clear, it's only the naming/calling conventions of the "return new PVector"
methods that I'd suggest need some consideration. Currently, those are: add(), sub
(), mult(), div(), cross().

Though, in addition, if you do adopt a opNew() approach, a couple of "in place only"
methods should be "upgraded" to offer an opNew() version in case you WANT to work
like JCM and leave the original intact: normalize(), limit().

A related side issue is that everything that implies a PVector result should return
PVector. Even the "operate internally" versions (they'd return "this"). So all
operations can be chained in a single statement, fe: a.normalize().cross(x); (i'd
call this a side issue, because this could be done in isolation even if nothing else was)

And those couple extra constructors, just for convenience for those of us who tend to
forget about get() :-D.

I CAN offer critique on static methods that return float though...

For example, why is there a static float dist(PVector v1,PVector v2)? In order to call
that successfully you MUST have instances, so either v1.dist(v2) or v2.dist(v1) would
have worked instead, at least in every case that I can imagine. The static dist()
does NOT check for nulls and create a zero-vector in its place - at first that's what i
thought the intent might have been, but nope. Same with static dot() and
angleBetween().

(btw, it doesn't really matter, but i notice a couple parenthetical math typos in my
initial post, just in case anyone ever tries to validate between the two code blocks)

cheers, and merry xmas!!
Additional Comment #3 From fry 2009-12-24 08:56
Sure, this is very helpful. I'm gonna disappear for a few days but will
give it more thought in the new year. It'd be nice to get PVector in better
shape, and I think it's not much work, it's just a matter of being a little
smarter about it.

Merry Christmas to you as well! Take care.
This bug is now being tracked here.