GoogleCodeArchive / piccolo2d

Automatically exported from code.google.com/p/piccolo2d
0 stars 0 forks source link

More consistent handling of NonInvertible transforms in PCamera #96

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
PCamera has a number of methods that transform from the local coordinate 
system to the view's by inverting the view transform and applying it to the 
input.

One of those methods catches a NoninvertibleTransformException and returns 
null, but the others ignore the problem completely. Since I think this is a 
valid case.

How should we be handling these cases:
1. Allowing the exception to bubble up
2. catching it and returning null to the localToView request

Personally I'd go with #1, but I'll wait to see what everyone else thinks.

Original issue reported on code.google.com by allain.lalonde on 14 Jul 2009 at 3:50

GoogleCodeExporter commented 9 years ago
I'll take no response as agreement since this seems to be something that few 
care 
enough about to answer about. I'll wait a little longer and then make the 
changes if 
there's not response.

Original comment by allain.lalonde on 16 Jul 2009 at 5:42

GoogleCodeExporter commented 9 years ago
Since in the vast majority of cases all Piccolo2D transforms are invertible, 
I'm going 
to go ahead and make it throw exceptions in all cases where it can't.  Figure 
it's more 
important that the developer be aware of this case than it is to insulate them 
from the 
reality of what might be a serious bug.

Original comment by allain.lalonde on 17 Jul 2009 at 2:18

GoogleCodeExporter commented 9 years ago
NoninvertibleTransformException is not a RuntimeException, so I'm going to wrap 
it with 
a custom exception that is. If I don't do this, method signatures would need to 
be 
changed.  This would break Binary compatability and make using any of those 
methods 
100x more annoying.

Original comment by allain.lalonde on 17 Jul 2009 at 2:24

GoogleCodeExporter commented 9 years ago
Why not wrapping it with just a plain RuntimeException?

Original comment by mr0...@mro.name on 17 Jul 2009 at 3:18

GoogleCodeExporter commented 9 years ago
Because it's generally considered to be a code smell (not being a smart ass).

Also, it's useful to do so since we can make the errant transform accessible 
from the 
exception.  So, if a coder wants to handle the exception more appropriately 
than just 
to bury it, they can examine the transform and respond.

Also, it makes it easier to distinguish the type of the exception without 
having to 
examine the exceptions properties. So, we don't treat divide by zero 
accidentally as 
a transform exception.

Original comment by allain.lalonde on 17 Jul 2009 at 3:56

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Fixed in r487; Moved non-invertible exception code into PAffineTransform, 
cleaning up 
other code in the process. 2 printStackTrace buried exceptions were removed.

Original comment by allain.lalonde on 17 Jul 2009 at 4:01

GoogleCodeExporter commented 9 years ago
uh - that's a drag:
http://java.sun.com/j2se/1.5.0/docs/api/java/awt/geom/NoninvertibleTransformExce
ption.html
doesn't tell nothing about the trafo. So a custom one makes sense, yes.

And eager checking sounds also good to me.

Original comment by mr0...@mro.name on 17 Jul 2009 at 5:02

GoogleCodeExporter commented 9 years ago

Original comment by heue...@gmail.com on 28 Jul 2009 at 1:35