Re: [HAPI-devel] Some other questions on PipeParser.getCriticalResponseData()

[Life is h. a. t. y. d. <ro...@in...>]

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