ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Conor MacNeill <co...@cortexebusiness.com.au>
Subject ProjectHelper comments
Date Mon, 11 Mar 2002 13:27:43 GMT
I've done cleanup on the ProjectHelper changes and I have some 
comments/questions. I'm not sure how much a pluggable ProjectHelper 
implementation is required.

1. I don't know why ProjectHelperImpl extends ProjectHelper. I had expected some 
sort of separate abstract class or interface that the ProjectHelperImpl would 
extend and which other ProjectHelperImpls would also extend. Right now 
ProjectHelper is acting as both the factory and base class for ProjectHelpers. I 
think it would be better to separate this out. Since the previuous 
implementation of ProjectHelper kept most methods private this should not be 
difficult.

2. Why has parse become public? There is no need for this to be public. In fact 
the old ProjectHelper only exposed configureProject (beside the utility methods)

3. Why is getProjectHelper public? There is no need for users of Projecthelper 
to get the actual ProjectHelper instance being used. All they should be using is 
  configureProject()

4. Why is ProjectHelper's constructor public?

5. If the user has specifically specified a Projecthelper instance via a 
property I don't think it should be ignored. The following should probably be fatal.
         } catch (SecurityException e) {
             // It's ok, we'll try next option
             ;
         }

6. It would be good to comment why you are ignoring this exception
             } catch (Exception ex) {
                 ;
             }
Probably logging at DEBUG level would be a better idea so users who plonk a jar 
into the lib directory understand why the project helper implementation is not 
being picked up.

Conor


--
To unsubscribe, e-mail:   <mailto:ant-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:ant-dev-help@jakarta.apache.org>


Mime
View raw message