You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by he...@apache.org on 2004/09/20 11:48:40 UTC

cvs commit: jakarta-commons/configuration/src/java/org/apache/commons/configuration ConfigurationMap.java ConfigurationSet.java

henning     2004/09/20 02:48:40

  Modified:    configuration/src/java/org/apache/commons/configuration
                        ConfigurationMap.java ConfigurationSet.java
  Log:
  Map is public, Set is package private?!?
  
  Also cleaned up the style (we should at some point converge onto one
  style. checkstyle uses the Turbine version, so the Turbine style
  (which is BTW broken in maven 1.0) is it currently).
  
  The Map implementation is very incomplete, The set implementation has
  at least one huge performance penalty.
  
  Revision  Changes    Path
  1.4       +21 -5     jakarta-commons/configuration/src/java/org/apache/commons/configuration/ConfigurationMap.java
  
  Index: ConfigurationMap.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/configuration/src/java/org/apache/commons/configuration/ConfigurationMap.java,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -r1.3 -r1.4
  --- ConfigurationMap.java	20 Sep 2004 09:37:07 -0000	1.3
  +++ ConfigurationMap.java	20 Sep 2004 09:48:40 -0000	1.4
  @@ -26,14 +26,19 @@
    * {@link org.apache.commons.configuration.Configuration}
    * instance to provide a <code>Map</code> interface.</p>
    *
  + * @todo This implementation is incomplete. 
  + *
    * @author <a href="mailto:ricardo.gladwell@btinternet.com">Ricardo Gladwell</a>
    */
  -public class ConfigurationMap extends AbstractMap {
  +
  +public class ConfigurationMap
  +        extends AbstractMap
  +{
   
       /**
        * The <code>Configuration</code> wrapped by this class.
        */
  -    Configuration configuration;
  +    Configuration configuration = null;
   
       /**
        * Creates a new instance of a <code>ConfigurationMap</code>
  @@ -42,24 +47,35 @@
        * @param configuration <code>Configuration</code>
        * instance.
        */
  -    public ConfigurationMap(Configuration configuration) {
  +    public ConfigurationMap(Configuration configuration)
  +    {
           this.configuration = configuration;
       }
   
       /**
        * @see java.util.Map#entrySet()
        */
  -    public Set entrySet() {
  +    public Set entrySet()
  +    {
           return new ConfigurationSet(configuration);
       }
   
       /**
        * @see java.util.Map#put(java.lang.Object, java.lang.Object)
        */
  -    public Object put(Object key, Object value) {
  +    public Object put(Object key, Object value)
  +    {
           Object old = configuration.getProperty((String) key);
           configuration.setProperty((String) key,value);
           return old;
  +    }
  +
  +    /**
  +     * @see java.util.Map#get(java.lang.Object)
  +     */
  +    public Object get(Object key)
  +    {
  +        return configuration.getProperty((String) key);
       }
   
   }
  
  
  
  1.5       +52 -36    jakarta-commons/configuration/src/java/org/apache/commons/configuration/ConfigurationSet.java
  
  Index: ConfigurationSet.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/configuration/src/java/org/apache/commons/configuration/ConfigurationSet.java,v
  retrieving revision 1.4
  retrieving revision 1.5
  diff -u -r1.4 -r1.5
  --- ConfigurationSet.java	20 Sep 2004 09:37:07 -0000	1.4
  +++ ConfigurationSet.java	20 Sep 2004 09:48:40 -0000	1.5
  @@ -24,25 +24,34 @@
   /**
    * @author <a href="mailto:ricardo.gladwell@btinternet.com">Ricardo Gladwell</a>
    */
  -class ConfigurationSet extends AbstractSet {
   
  -    class Entry implements Map.Entry {
  +public class ConfigurationSet
  +        extends AbstractSet
  +{
  +    private Configuration configuration = null;
  +
  +    public class Entry
  +            implements Map.Entry
  +    {
  +        private Object key = null;
   
  -        Object key;
  -
  -        public Entry(Object key) {
  +        public Entry(Object key)
  +        {
               this.key = key;
           }
   
  -        public Object getKey() {
  +        public Object getKey()
  +        {
               return key;
           }
   
  -        public Object getValue() {
  +        public Object getValue()
  +        {
               return configuration.getProperty((String) key);
           }
   
  -        public Object setValue(Object value) {
  +        public Object setValue(Object value)
  +        {
               Object old = getValue();
               configuration.setProperty((String) key, value);
               return old;
  @@ -50,52 +59,59 @@
   
       }
   
  -    class ConfigurationSetIterator implements Iterator {
  -
  -        Iterator keys;
  -
  -        public ConfigurationSetIterator() {
  -        	keys = configuration.getKeys();
  +    public class ConfigurationSetIterator
  +            implements Iterator
  +    {
  +        private Iterator keys;
  +
  +        public ConfigurationSetIterator()
  +        {
  +            keys = configuration.getKeys();
           }
   
  -        public boolean hasNext() {
  +        public boolean hasNext()
  +        {
               return keys.hasNext();
  -         }
  +        }
   
  -        public Object next() {
  +        public Object next()
  +        {
               return new Entry(keys.next());
  -         }
  +        }
   
  -         public void remove() {
  -         	keys.remove();
  -         }
  +        public void remove()
  +        {
  +            keys.remove();
  +        }
   
       }
   
  -    Configuration configuration;
  -
  -    public ConfigurationSet(Configuration configuration) {
  +    public ConfigurationSet(Configuration configuration)
  +    {
           this.configuration = configuration;
       }
   
       /**
  -	 * @see java.util.Collection#size()
  -	 */
  -	public int size() {
  +     * @see java.util.Collection#size()
  +     */
  +    public int size()
  +    {
  +        // Ouch. Now _that_ one is expensive...
           int count = 0;
  -		Iterator iterator = configuration.getKeys();
  -        while(iterator.hasNext()) {
  +        for (Iterator iterator = configuration.getKeys(); iterator.hasNext(); )
  +        {
               iterator.next();
               count++;
           }
           return count;
  -	}
  +    }
   
  -	/**
  -	 * @see java.util.Collection#iterator()
  -	 */
  -	public Iterator iterator() {
  -		return new ConfigurationSetIterator();
  -	}
  +    /**
  +     * @see java.util.Collection#iterator()
  +     */
  +    public Iterator iterator()
  +    {
  +        return new ConfigurationSetIterator();
  +    }
   
   }
  
  
  

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


RE: [configuration] Set is package private?!?

Posted by Eric Pugh <ep...@upstate.com>.
b/c inner classes are icky and make my head hurt :-)   No, I don't have a
specific urge to keep it seperate...

eric

> -----Original Message-----
> From: Henning P. Schmiedehausen [mailto:hps@intermeta.de]
> Sent: Monday, September 20, 2004 6:54 PM
> To: commons-dev@jakarta.apache.org
> Subject: Re: [configuration] Set is package private?!?
>
>
> Emmanuel Bourg <sm...@lfjr.net> writes:
>
> >Henning P. Schmiedehausen wrote:
>
> >> Is it? I agree that I see no obvious usage for it, but our users have
> >> been proven to be imaginative. Else it should (IMHO) either be an
> >> inner class to ConfigurationMap or at least moved to a sub-package.
> >>
> >> 	Regards
> >> 		Henning
>
> >This class is really an implementation detail, as a package private
> >class or private (anonymous?) inner class I have no preference, but not
> >public.
>
> Ok. Any objections for moving it as an inner class into ConfigurationMap ?
>
> 	Regards
> 		Henning
>
> --
> Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
> hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/
>
> RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
>    Linux, Java, perl, Solaris -- Consulting, Training, Development
>
> "Fighting for one's political stand is an honorable action, but re-
>  fusing to acknowledge that there might be weaknesses in one's
>  position - in order to identify them so that they can be remedied -
>  is a large enough problem with the Open Source movement that it
>  deserves to be on this list of the top five problems."
>                        -- Michelle Levesque, "Fundamental Issues with
>                                     Open Source Software Development"
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org


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


Re: [configuration] Set is package private?!?

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Emmanuel Bourg <sm...@lfjr.net> writes:

>Henning P. Schmiedehausen wrote:

>> Is it? I agree that I see no obvious usage for it, but our users have
>> been proven to be imaginative. Else it should (IMHO) either be an
>> inner class to ConfigurationMap or at least moved to a sub-package.
>> 
>> 	Regards
>> 		Henning

>This class is really an implementation detail, as a package private 
>class or private (anonymous?) inner class I have no preference, but not 
>public.

Ok. Any objections for moving it as an inner class into ConfigurationMap ?

	Regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

"Fighting for one's political stand is an honorable action, but re-
 fusing to acknowledge that there might be weaknesses in one's
 position - in order to identify them so that they can be remedied -
 is a large enough problem with the Open Source movement that it
 deserves to be on this list of the top five problems."
                       -- Michelle Levesque, "Fundamental Issues with
                                    Open Source Software Development"

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


Re: [configuration] Set is package private?!?

Posted by Emmanuel Bourg <sm...@lfjr.net>.
Henning P. Schmiedehausen wrote:

> Is it? I agree that I see no obvious usage for it, but our users have
> been proven to be imaginative. Else it should (IMHO) either be an
> inner class to ConfigurationMap or at least moved to a sub-package.
> 
> 	Regards
> 		Henning

This class is really an implementation detail, as a package private 
class or private (anonymous?) inner class I have no preference, but not 
public.


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


Re: [configuration] Set is package private?!?

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Emmanuel Bourg <sm...@lfjr.net> writes:

>henning@apache.org wrote:

>> henning     2004/09/20 02:48:40
>> 
>>   Modified:    configuration/src/java/org/apache/commons/configuration
>>                         ConfigurationMap.java ConfigurationSet.java
>>   Log:
>>   Map is public, Set is package private?!?

>Yes it's package private because it's just used to implement the 
>keySet() method of ConfigurationMap, there is no reason to make it 
>public since it's useless for end users.

Is it? I agree that I see no obvious usage for it, but our users have
been proven to be imaginative. Else it should (IMHO) either be an
inner class to ConfigurationMap or at least moved to a sub-package.

	Regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

"Fighting for one's political stand is an honorable action, but re-
 fusing to acknowledge that there might be weaknesses in one's
 position - in order to identify them so that they can be remedied -
 is a large enough problem with the Open Source movement that it
 deserves to be on this list of the top five problems."
                       -- Michelle Levesque, "Fundamental Issues with
                                    Open Source Software Development"

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


[configuration] Set is package private?!?

Posted by Emmanuel Bourg <sm...@lfjr.net>.
henning@apache.org wrote:

> henning     2004/09/20 02:48:40
> 
>   Modified:    configuration/src/java/org/apache/commons/configuration
>                         ConfigurationMap.java ConfigurationSet.java
>   Log:
>   Map is public, Set is package private?!?

Yes it's package private because it's just used to implement the 
keySet() method of ConfigurationMap, there is no reason to make it 
public since it's useless for end users.

Emmanuel Bourg


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