forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thorsten Scherler <thorsten.scher...@wyona.com>
Subject Do not raise exception when you can prevent it (was Re: svn commit: r428677)
Date Fri, 04 Aug 2006 11:08:41 GMT
El vie, 04-08-2006 a las 09:44 +0000, rgardler@apache.org escribió:
> Author: rgardler
> Date: Fri Aug  4 02:44:24 2006
> New Revision: 428677
> 
> URL: http://svn.apache.org/viewvc?rev=428677&view=rev
> Log:
> add a guard around the loading of forrest.properties file. That file is no longer required
> 
> Modified:
>     forrest/trunk/main/java/org/apache/forrest/conf/ForrestConfModule.java
> 
> Modified: forrest/trunk/main/java/org/apache/forrest/conf/ForrestConfModule.java
> URL: http://svn.apache.org/viewvc/forrest/trunk/main/java/org/apache/forrest/conf/ForrestConfModule.java?rev=428677&r1=428676&r2=428677&view=diff
> ==============================================================================
> --- forrest/trunk/main/java/org/apache/forrest/conf/ForrestConfModule.java (original)
> +++ forrest/trunk/main/java/org/apache/forrest/conf/ForrestConfModule.java Fri Aug  4
02:44:24 2006
> @@ -166,16 +166,19 @@
>          }
>  
>          // get forrest.properties and load the values
> -        forrestPropertiesStringURI = projectHome + SystemUtils.FILE_SEPARATOR
> -                + "forrest.properties";
> -
> -        filteringProperties = loadAntPropertiesFromURI(filteringProperties,
> +        try {
> +            forrestPropertiesStringURI = projectHome + SystemUtils.FILE_SEPARATOR
> +                + "forrest.properties";        
> +            filteringProperties = loadAntPropertiesFromURI(filteringProperties,
>                  forrestPropertiesStringURI);
> +        } catch (FileNotFoundException e) {
> +            if (debugging())
> +                debug("Unable to find forrest.properties, using defaults.");
> +        }
>  
>          // get default-forrest.properties and load the values
>          String defaultForrestPropertiesStringURI = contextHome + SystemUtils.FILE_SEPARATOR
>                  + "default-forrest.properties";
> -
>          filteringProperties = loadAntPropertiesFromURI(filteringProperties,
>                  defaultForrestPropertiesStringURI);
>  
> @@ -201,7 +204,7 @@
>          loadSystemProperties(filteringProperties);
>          ForrestConfUtils.aliasSkinProperties(filteringProperties);
>          if (debugging())
> -            debug("Loaded project forrest.properties:" + filteringProperties);
> +            debug("Loaded project properties:" + filteringProperties);
>      }
>  
>      /**
> 

I am not sure about this change. 

I do not like that an exception is not prevented but catched. There are
now tons of try catch blocks that are not needed at all if we use a
cleaner solution.

IMO it is way cleaner to test whether the source.exist() before trying
to read form it. This way a FileNotFoundException is prevented.

Index: main/java/org/apache/forrest/conf/ForrestConfModule.java
===================================================================
--- main/java/org/apache/forrest/conf/ForrestConfModule.java
(revisión: 428691)
+++ main/java/org/apache/forrest/conf/ForrestConfModule.java    (copia
de trabajo)
@@ -313,12 +313,14 @@

             if (debugging())
                 debug("Searching for forrest.properties in" +
source.getURI());
-            in = source.getInputStream();
-            filteringProperties = new
AntProperties(precedingProperties);
-            filteringProperties.load(in);
+            if (source.exists()){
+               in = source.getInputStream();
+                filteringProperties = new
AntProperties(precedingProperties);
+                filteringProperties.load(in);

-            if (debugging())
-                debug("Loaded:" + antPropertiesStringURI +
filteringProperties.toString());
+                if (debugging())
+                    debug("Loaded:" + antPropertiesStringURI +
filteringProperties.toString());
+            }

I will change this, if nobody objects. This includes removing all try
catches surrounding the calling code.

Further I would like to add this to the coding guidelines, that
exception should be prevented and not using them for debugging.


salu2
-- 
Thorsten Scherler
COO Spain
Wyona Inc.  -  Open Source Content Management  -  Apache Lenya
http://www.wyona.com                   http://lenya.apache.org
thorsten.scherler@wyona.com                thorsten@apache.org


Mime
View raw message