qzindustries / qz-print

Free browser applet for sending documents and raw commands to a printer.
Other
49 stars 4 forks source link

Dispose Jframe on Printer Error #41

Closed mmadrigalac closed 10 years ago

mmadrigalac commented 10 years ago

Added thrown exception and dispose of JFrame in order to avoid incorrect Java popup display/ behavior

tresf commented 10 years ago

Thanks!

I made some comments on the commit. Subsequent changes you make to your branch will automatically be reflected in this pull request until it is approved.

mmadrigalac commented 10 years ago

I decided to apply Finally sentence. Please take a look :)

tresf commented 10 years ago

I decided to apply Finally sentence. Please take a look :)

I'm afraid this would never be reached because you raise a new throw statement, which would break out of the try/catch/finally. With that mentality, you would probably use finally to release the resource, then throw it up the stack... perhaps:

Exception e = null;

try {}
catch (e) {}
finally {
   j.dispose();
}

// Break out if e was caught
if (e != null) { throw e; }

Does that make more sense? P.S. This approach may be wrong. Feel free to correct me.

-Tres

mmadrigalac commented 10 years ago

I did test the change setting several scenarios and in all of them finally sentence worked fine, even if an Exception what was handled correctly by PrinterException (Java console shows the error and I was able to catch it with getException() as expected) or even if print process was finished correctly, So I considered this as a good approach after all.

Let me know if you want me to change it in order to follow any standard you use. Best Regards!