|
From: Life is h. a. t. y. d. <ro...@in...> - 2006-07-26 06:59:50
|
On Tue, Jul 25, 2006 at 04:57:34PM +0200, Rick Riemer wrote: > > Once more, I've got some questions on the > PipeParser.getCriticalResponseData() method. The method contains a catch > (Exception e) block, which print a stack trace for the caught exception > and throws a new one. Three things come to mind here: > * Shouldn't catch (Exception e) really be a catch (specific > exception), such as catch (HL7Exception e)? Catching all exceptions is > typically a bad idea (see: > http://www.javaworld.com/javaworld/javatips/jw-javatip134.html or > http://javaalmanac.com/egs/Java%20Language/CatchThrowable.html?l=rel) > * Can the e.printStackTrace() be removed, in favor of having the > caller handle the exception? A library should typically not print > messages to the console. > * Can the caught exception be specified as nested exception of the > newly created HL7Exception? Oh right, I remember patching this too... I used the following trivial fix: ------------------------------------------------------------------------- --- ca/uhn/hl7v2/parser/PipeParser.java +++ ca/uhn/hl7v2/parser/PipeParser.java @@ -639,7 +639,6 @@ Terser.set(msh, 11, 0, 1, 1, procIDComps[0]); } catch (Exception e) { - e.printStackTrace(); throw new HL7Exception( "Can't parse critical fields from MSH segment (" + e.getClass().getName() @@ -647,7 +646,7 @@ + e.getMessage() + "): " + mshString, - HL7Exception.REQUIRED_FIELD_MISSING); + HL7Exception.REQUIRED_FIELD_MISSING, e); } return msh; ------------------------------------------------------------------------- Which covers your points 2 and 3. Cheers, Ronald |