Eliminate printStackTrace

[GoogleCodeExporter]

What steps will reproduce the problem?

$ find . -name "*.java" | xargs grep printStackTrace

What is the expected output? What do you see instead?

expected: 0 hits

found:

./core/src/main/java/edu/umd/cs/piccolo/PNode.java:           
e.printStackTrace();
./core/src/main/java/edu/umd/cs/piccolo/PNode.java:           
e.printStackTrace();
./core/src/main/java/edu/umd/cs/piccolo/PNode.java:           
e.printStackTrace();
./core/src/main/java/edu/umd/cs/piccolo/PNode.java:               
e.printStackTrace();
./examples/src/main/java/edu/umd/cs/piccolo/examples/PrintExample.java:   
            e.printStackTrace();
./examples/src/main/java/edu/umd/cs/piccolo/examples/swt/SWTBenchTest.java:
           e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/event/PNotificationCenter.java:
           e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/event/PNotificationCenter.java:
                   e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/event/PNotificationCenter.java:
                   e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/nodes/PStyledText.java:        
   e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/pswing/PSwingEventHandler.java:
           e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/pswing/PSwingEventHandler.java:
                       + sourceSwingEvent + ", class=" +
sourceSwingEvent.getClass().getName()).printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/pswing/PSwingEventHandler.java:
        * ).printStackTrace(); } if( aEvent.isMouseEvent() &&
./swt/src/main/java/edu/umd/cs/piccolox/swt/PSWTPath.java:            new
Exception().printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/SWTGraphics2D.java:           
e.printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/SWTGraphics2D.java:           
e.printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/SWTGraphics2D.java:           
    e.printStackTrace();

Original issue reported on code.google.com by mr0...@mro.name on 18 Jul 2009 at 1:00

[GoogleCodeExporter]

I'd like to replace them with "throw new RuntimeException(e);" - objections?

Original comment by mr0...@mro.name on 18 Jul 2009 at 1:01

[GoogleCodeExporter]

wow that's a lot of them.

Objection would be that RuntimeException is indistinguishable from a Divide by 
zero
exception, or any number of other system level exceptions.

I think there should be a PPiccoloException to differentiate between exceptions
caused by the subsystem and exceptions caused by Piccolo.

Also, without examining the context for each printStackTrace, blanket replacing 
them
with exceptions is bound to not work.

Original comment by allain.lalonde on 18 Jul 2009 at 1:22

[GoogleCodeExporter]

I'd attach the caught Exception as nested one to the RTE - so there's no 
information
loss.

Original comment by mr0...@mro.name on 18 Jul 2009 at 1:24

[GoogleCodeExporter]

It's considered an important enough code smell that it's part of PMD's default 
checklist.

The context of the printStackTrace is important before replacing since the 
system is
now running with the expected behaviour being to bury the exception. Changing 
that
behaviour may introduce bugs in client software.

I absolutely think they should be dealt with, just on an individual basis.

Original comment by allain.lalonde on 18 Jul 2009 at 1:29

[GoogleCodeExporter]

I vote against a generic, non-abstract PPiccoloException - if required we 
should use
fine-grained ones with clear semantic meaning (as discussed with the 
NonInvertible
topic).

Original comment by mr0...@mro.name on 18 Jul 2009 at 1:59

[GoogleCodeExporter]

It's a little shorter now as per r516

./extras/src/main/java/edu/umd/cs/piccolox/event/PNotificationCenter.java:      

         e.printStackTrace();
./extras/src/main/java/edu/umd/cs/piccolox/pswing/PSwingEventHandler.java:      

             + sourceSwingEvent + ", class=" +
sourceSwingEvent.getClass().getName()).printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/SWTGraphics2D.java:           
e.printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/SWTGraphics2D.java:           
e.printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/SWTGraphics2D.java:               
e.printStackTrace();
./swt/src/main/java/edu/umd/cs/piccolox/swt/PSWTPath.java:            new
Exception().printStackTrace();
./examples/src/main/java/edu/umd/cs/piccolo/examples/swt/SWTBenchTest.java:     

  e.printStackTrace();
./core/src/main/java/edu/umd/cs/piccolo/PNode.java:                
e.printStackTrace();

Original comment by allain.lalonde on 19 Jul 2009 at 12:45

[GoogleCodeExporter]

Also do it on Sat, 12:00-14:00 UTC.

Original comment by mr0...@mro.name on 23 Jul 2009 at 6:06

[GoogleCodeExporter]

The first two are the topmost spot in an event dispatch loop, Swing using the
printStackTrace behaviour when exceptions bubble up to the top, so I think this 
is
one of those rare instances where there's not much else you should do.  
Eliminating
them, without having something better should be better. I'm all ears.

the printStackTrace in PNode can be dealt with by dealing with Issue 102, I 
don't
know of a more sensible thing than just printing the stack trace if you want to
remain binary compatible. You can't very well open up a Dialog box.

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

[GoogleCodeExporter]

Wow, grammar on that last one was terrible. And that's my cue to stop coding. 
'nite.

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

[GoogleCodeExporter]

please verify and vote r562.

Original comment by mr0...@mro.name on 25 Jul 2009 at 1:30

• Changed state: Fixed

[GoogleCodeExporter]

undone in r596. will commit the agreed on changes soon.

Original comment by mr0...@mro.name on 28 Jul 2009 at 7:08

• Changed state: Started

[GoogleCodeExporter]

redone in r601. Please verify & vote.

Original comment by mr0...@mro.name on 28 Jul 2009 at 8:42

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by heue...@gmail.com on 30 Oct 2009 at 3:05

• Changed state: Verified