You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lenya.apache.org by Andreas Hartmann <an...@apache.org> on 2006/08/03 21:36:11 UTC

ModuleManager.getModueList (was: svn commit: r428420)

Hi Thorsten,

thanks for this function!
Some little comments:

thorsten@apache.org wrote:
> Author: thorsten
> Date: Thu Aug  3 08:13:09 2006
> New Revision: 428420
> 
> URL: http://svn.apache.org/viewvc?rev=428420&view=rev
> Log:
> Extending the module API with a method to return a list of modules and their location. Adding a default implementation as well.
> 
> Modified:
>     lenya/trunk/src/impl/java/org/apache/lenya/cms/module/ModuleManagerImpl.java
>     lenya/trunk/src/java/org/apache/lenya/cms/module/ModuleManager.java

[...]

> +    public Map getModuleList(){
> +        return module2src;


* We should never return a collection field, because it can be
   modified by client code. To make this method safe, an (unmodifiable)
   copy of the map has to be created:

   return Collections.unmodifiableMap(this.module2src);

* IMO a method called "get...List" shouldn't return a map.

* If a map is returned, the javadocs should explain what the
   map contains (key=..., value=...)

* IMO the most obvious signature would be to return an array, e.g.

String[] getModuleIds();
String getModuleSourceUri(String moduleId);

In this case, you don't need the javadocs to understand the methods.

WDYT?

-- Andreas



-- 
Andreas Hartmann
Wyona Inc.  -   Open Source Content Management   -   Apache Lenya
http://www.wyona.com                      http://lenya.apache.org
andreas.hartmann@wyona.com                     andreas@apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: ModuleManager.getModueList (was: svn commit: r428420)

Posted by Thorsten Scherler <th...@wyona.com>.
El vie, 04-08-2006 a las 09:24 +0200, Thorsten Scherler escribió:
> El jue, 03-08-2006 a las 21:36 +0200, Andreas Hartmann escribió:
...
> > String[] getModuleIds();
> > String getModuleSourceUri(String moduleId);
> > 
> > In this case, you don't need the javadocs to understand the methods.
> > 
> > WDYT?
> 
> I will change the method ASAP.

I only added getModuleIds() since getModuleSourceUri(...) is already in
there as getBaseURI(...).

Thanks for your feedback.

salu2

> 
> 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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org


Re: ModuleManager.getModueList (was: svn commit: r428420)

Posted by Thorsten Scherler <th...@apache.org>.
El jue, 03-08-2006 a las 21:36 +0200, Andreas Hartmann escribió:
> Hi Thorsten,
> 
> thanks for this function!
> Some little comments:
> 
> thorsten@apache.org wrote:
> > Author: thorsten
> > Date: Thu Aug  3 08:13:09 2006
> > New Revision: 428420
> > 
> > URL: http://svn.apache.org/viewvc?rev=428420&view=rev
> > Log:
> > Extending the module API with a method to return a list of modules and their location. Adding a default implementation as well.
> > 
> > Modified:
> >     lenya/trunk/src/impl/java/org/apache/lenya/cms/module/ModuleManagerImpl.java
> >     lenya/trunk/src/java/org/apache/lenya/cms/module/ModuleManager.java
> 
> [...]
> 
> > +    public Map getModuleList(){
> > +        return module2src;
> 
> 
> * We should never return a collection field, because it can be
>    modified by client code. To make this method safe, an (unmodifiable)
>    copy of the map has to be created:
> 
>    return Collections.unmodifiableMap(this.module2src);
> 
> * IMO a method called "get...List" shouldn't return a map.
> 
> * If a map is returned, the javadocs should explain what the
>    map contains (key=..., value=...)
> 
> * IMO the most obvious signature would be to return an array, e.g.
> 
> String[] getModuleIds();
> String getModuleSourceUri(String moduleId);
> 
> In this case, you don't need the javadocs to understand the methods.
> 
> WDYT?

I will change the method ASAP.

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org