Re: cvs commit: jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic DateTool.java

[Daniel L. Rall]

Why are you catching Exception?  Nothing in that block throws Exception.  If 
later a checked exception is introduced into the main try block there, a 
developer may not realize it due to this overzealous exception supression. 
I'd much prefer that you revert that to catch RuntimeException -- its use was 
a conscious choice which preserves the semantics of the method.  Complete 
supression of exceptions (unchecked or otherwise) must be done with care.  :)

Returning null explicitly also changes the semantics of the method. 
Previously, you could've created a DateFormat instance, but not filled in its 
time zone information, and still had something to return even if an exception 
was thrown.  Personally, I much prefer the return of null when an error 
occurs, but a change in semantics demands more JavaDoc and/or a meaningful 
change log message.

- Dan

[EMAIL PROTECTED] wrote:
nbubna      2003/11/10 17:55:22

  Modified:    src/java/org/apache/velocity/tools/generic DateTool.java
  Log:
  the trivial change i actually meant to commit last time
  
  Revision  Changes    Path
  1.8       +6 -4      jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic/DateTool.java
  
  Index: DateTool.java
  ===================================================================
  RCS file: /home/cvs/jakarta-velocity-tools/src/java/org/apache/velocity/tools/generic/DateTool.java,v
  retrieving revision 1.7
  retrieving revision 1.8
  diff -u -r1.7 -r1.8
  --- DateTool.java	11 Nov 2003 01:16:54 -0000	1.7
  +++ DateTool.java	11 Nov 2003 01:55:21 -0000	1.8
  @@ -540,9 +540,9 @@
       protected DateFormat getDateFormat(int dateStyle, int timeStyle, 
                                          Locale locale, TimeZone timezone)
       {
  -        DateFormat df = null;
           try
           {
  +            DateFormat df;
               if (dateStyle < 0 && timeStyle < 0)
               {
                   // no style was specified, use default instance
  @@ -564,10 +564,12 @@
                                                       locale);
               }
               df.setTimeZone(timezone);
  +            return df;
           }
  -        catch (RuntimeException suppressed)
  +        catch (Exception suppressed)
           {
  -            return df;
  +            // let it go...
  +            return null;
           }
       }


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]