Affine transforms - matrix algebra

[Richard Bair]

On Aug 16, 2012, at 3:52 PM, Jim Graham wrote:

> On 8/16/2012 10:38 AM, Pavel Safrata wrote:
>> On 16.8.2012 16:29, Richard Bair wrote:
>>>> New NoninvertibleTransformException
>>> I believe it should be NonInvertibleTransformException with a capital
>>> "I".
>> 
>> Both in J2D and in our internal geom packages the "i" is lower-case. But
>> I have no problem changing it.
> 
> In particular, Non isn't a word so why should it be separated (via capitalization) from the word to which it is a prefix?

Well, noninvertible isn't a word (at least, according to my dictionary), and I don't know that non-invertible would be represented as Noninvertible when converted to camel-case?

>>>> public double determinant()
>>> Why isn't this a getter?
>> 
>> Because currently it is not a property.
> 
> I'm not sure one would want to track it or bind to it either.
> 
>>>> public boolean is2D()
>>>> public boolean is3D()
>>>> public boolean isIdentity()
>>> All four of these, determinant, is2D, is3D, and isIdentity can all (in
>>> theory) change their value as the Transform is changed, is that right?
>>> In that case, they should be read only properties.
>> 
>> Ah, they probably should. So:
> 
> Or should they?  What is the use case for tracking or binding to these?  You can already listen for events to know when things change.  These are all more for "I want to know how to most quickly handle this transform" and are only useful for optimization cases where you still need to write the general case.  Since you always need the general case and since these attributes are only needed when you actually get to the "I'm dealing with the transform values now" code, why do they need to be properties?
> 
> I think properties should be things that are independently specifiable.  Values that you calculate from other properties shouldn't necessarily be properties themselves (though it might make sense in some cases - you should provide a use case to argue for that rather than just have a "wouldn't it be great if everything were a property" default approach).
> 
> Why don't we make hashcode a property?  ;)

In fact, it is! hashcode follows precisely the pattern for immutable properties -- it is just a getter. If something can change, somebody is going to want to know about it. Listening to changes via an event object is OK if you don't care what exactly has changed, but cumbersome when you want to know exactly what has changed. In general though, the issue I have is that a getFoo() should either be immutable, or should have a fooProperty() method. The reason I have wanted to maintain this pattern is that it allows tools / frameworks to deduce things about properties on objects by the patterns they follow.

Alternatively, we could (I suppose) have taken to annotating everything to indicate whether they are mutable / immutable etc. The way we've worked around this in other places where we didn't want something to be observable was to just not prefix it with get / is, but like with List use a simple name, such as size(). So "determinant" is OK if we decide we don't want to make it observable for terms of weight. The lame part about such names though is that some names like "identity" are not clear as to whether they are reading the identity or making it an identity transform.

>>> with the array as the last parameter, it also means it could be a
>>> double... if you wanted to, but I'm not sure it matters.
> 
> With regard to using the varargs notation for the setters:
> 
> I think that would be interesting and fun.  On the other hand, we might be better served by having methods that explicitly take the N parameters since we then avoid unnecessary object creation.  Also, these methods do not take an arbitrary number of doubles, they take a very explicit number of doubles based on the matrix type they are considering so the flexibility of the varargs notation is a little overkill when it is really just a lower performing way to allow inline argument list syntax.

If you're going to provide an array, there is no difference in performance or semantics between varargs or []. Avoiding an array all together (and, ostensibly the MatrixType argument and offset argument) at least has the benefit of not requiring any bounds checking etc within the constructor, so I can see an argument for having only that.

I can see Pavel tearing his hair out though. This is the longest API review thread ever!

>> First, this is not only issue of the constructor, but of all the methods
>> that accept the array of doubles. I was thinking about the order too. I
>> chose this one because the beginIndex is insignificant and in a vast
>> majority of cases it will be 0, so it seemed strange to have it before
>> the actual important data. Also all the methods where the type is first
>> are kind of getters, you just pass in what you want to get, whereas to
>> the kind of setters you pass the data and than tell how to interpret
>> them, so it is not really inconsistent. So your order takes a bit of
>> peace from my heart, but I don't insist on the proposed order. Any other
>> opinions?
> 
> Unfortunately I don't have a strong opinion there either, but if potential varargs usage is one of the key reasons for moving the array to the end, then I think I've given some good counter-reasons above...

Having parameters in a more-or-less consistent order was really what I was after. Varargs just fell out of it, but wasn't a really useful reason in and of itself.

Richard