Re: Some nice small graphics demo programs

On 8/14/2014 8:17 PM, Antti J Ylikoski wrote:


     Everyone else is going on and on about formatting, so I thought
I'd offer a few remarks about the stuff other than white space. I've
only looked at the Hilbert curves code, so my observations may or may
not be relevant to the other things.

     First: Put your code in named packages, and don't leave it in the
default "unnamed" package. It might sort of almost be all right to use
the unnamed package for programs whose code is completely contained in
one single source file, but as soon as you've got two or more files you
really, really, really ought to package them. Seriously. It will save
you a lot of headaches.

     Second: Swing is not thread-safe, so nearly every manipulation of a
Swing object must occur on the Event Dispatch Thread. You'll find some
old tutorials and examples that construct and initialize entire GUI's
directly from main(), but that's because Sun themselves didn't realize
at first how pernicious the race conditions could be. A sample of how
to set up a GUI might look something like

     public static void main(String[] args) {
         SwingUtilities.invokeLater(new Runnable(
             // This method will be called on the EDT:
             @Override
             public void run() {
                 // All the Swing setup happens here:
                 JFrame frame = new JFrame("Gooey");
                 ...
                 frame.pack();
                 frame.setVisible(true);
             }
         ));
         ...

(I've shown the "traditional Java" form; Java 8's "lambda expressions"
offer a sleeker way to write this.)

     Third: Swing is not thread-safe, et cetera et cetera, so if you're
writing classes that extend or combine Swing components there will be
parts of your own code that must run on the EDT and you should take
steps to ensure that your class is used correctly by other classes.
For example, if your class extends JPanel (quite a common thing to do
in Swing applications), its constructor, like JPanel's, should only be
invoked on the EDT. Here's an idiom I find helpful:

     public class DrawPanel extends JPanel {
         public DrawPanel(...) {
             super(...);
             assert SwingUtilities.isEventDispatchThread();
             ...
         }
         ...
     }

There's no clean way to do the assert prior to invoking the superclass
constructor, but putting it just as early as possible will catch a lot
of careless errors during development instead of after deployment.

     My remaining comments are about opportunities to streamline, clean
up, or otherwise improve the DrawPanel class.

     Formally, the paintComponent() method receives a Graphics reference.
However, what Swing actually passes is a reference to a Graphics2D, a
Graphics subclass with additional capabilities. Among these are the
ability to establish a mapping between user-space and display-space
coordinates: You can translate, scale, rotate, or apply any arbitrary
affine transform. In brief, all that work you're doing to derive pixel
coordinates could be done for you by the Graphics2D methods.

     ... and done more accurately, in all probability. You've taken a
rather cavalier approach when converting floating-point to int values,
just chopping off any fractional part that might be present. This will
get you a value, sure, but not always the best value. You may "know"
that the result of some calculation should be 42, exactly, but because
floating-point calculations involve approximation the value you actually
compute might be 41.99999999932165357 instead. When you convert such a
number to int, you should consider whether to chop or round (probably
with one of the Math.round() methods). Graphics2D's mapping of user
coordinates to pixel coordinates is careful about such things.

     Floating-point jitter also shows up in your intPower() method,
which I reproduce here for those who might not have seen it:

     private static int intPower(int n, int e) // raise int to int power
     {
     return (int) Math.pow((double)n, (double)e);
     } // end method intPower

(The (double) casts are unnecessary, by the way: Java knows the argument
types for Math.pow(), and will do the conversions without being told.)
The problem, again, is the approximate nature of floating-point: You may
call intPower(7,3) and "know" that the result should be 343, but the
computed value might be 343.0000000000003455 or 342.99999999992353256.
In the latter case, the naive conversion to int could surprise you by
raising an odd prime to an integer power and getting an even number!
Again, Math.round() would be a countermeasure.

     A closer look, though: When your code calls intPower(), n is always
equal to 2 -- you never try to exponentiate any other value! There's
an easier and more straightforward way to raise 2 to an integer power:

     private static int twoToThePower(int e) {
         return 1 << e;
     }

     But a still closer look suggests the method is entirely unnecessary!
It's only called once, in this loop:

     for(nn = 1; nn <= n; nn++) {
         scale = intPower(2, nn+1);
         ...

That is, scale takes on the values 4,8,16,... in successive iterations.
You could just calculate those values as they come up, instead of going
through an entire "from scratch" exponentiation each time:

     for (scale = 4, nn = 1; nn <= n; scale *= 2, nn++) ...

(Compiler implementors call this "strength reduction.")