Eric, I have created the patch for refactoring the addProperty() method. AbstractConfiguration.addProperty() now only handles collection and string properties with multiple values and then delegates to the abstract addPropertyDirect() method.
The stuff with the containers was moved to BaseConfiguration.addPropertyDirect(). The unit tests run fine. There is a little thing more I changed: I made the nested Container class in AbstractConfiguration static. The reason is that I have a static nested class HierarchicalProperties.Node that needs to create instances of Container. This is impossible with Container being nonstatic unless I make the Node class nonstatic, too; but I don't like that because it would make usage of this class more complicated for clients of HierarchicalProperties (normally clients need not access this class directly, but there might be cases where it could make sense). Was there any specific reason for making Container nonstatic? Regards Oli ----- Original Message ----- From: "Eric Pugh" <[EMAIL PROTECTED]> To: "'Oliver Heger'" <[EMAIL PROTECTED]>; "'Jakarta Commons Developers List'" <[EMAIL PROTECTED]> Sent: Tuesday, October 14, 2003 10:40 AM Subject: RE: [configuration]HierarchicalConfiguration > Oliver, > > So, if I understand properly, the reason is so that we refactor out the code > so that you don't have to do a cut'n'paste job from the > AbstractConfiguration to your new subclass.. > > Seems reasonable enough.. Why don't you submit a patch with your next chunk > of code then? > > Oh, and the attachements came through okay for me! > > Eric >
Index: src/java/org/apache/commons/configuration/AbstractConfiguration.java =================================================================== RCS file: /home/cvspublic/jakarta-commons-sandbox/configuration/src/java/org/apache/commons/configuration/AbstractConfiguration.java,v retrieving revision 1.2 diff -u -r1.2 AbstractConfiguration.java --- src/java/org/apache/commons/configuration/AbstractConfiguration.java 12 Oct 2003 09:32:30 -0000 1.2 +++ src/java/org/apache/commons/configuration/AbstractConfiguration.java 14 Oct 2003 10:37:35 -0000 @@ -69,6 +69,7 @@ * then you should implement only abstract methods from this class. * * @author <a href="mailto:[EMAIL PROTECTED]">Konstantin Shaposhnikov</a> + * @author <a href="mailto:[EMAIL PROTECTED]">Oliver Heger</a> * @version $Id: AbstractConfiguration.java,v 1.2 2003/10/12 09:32:30 rdonkin Exp $ */ public abstract class AbstractConfiguration implements Configuration @@ -124,11 +125,13 @@ */ public void addProperty(String key, Object token) { - List tokenAdd = null; - if (token instanceof String) { - tokenAdd = processString((String) token); + for(Iterator it = processString((String) token).iterator(); + it.hasNext();) + { + addPropertyDirect(key, it.next()); + } } else if (token instanceof Collection) { @@ -136,59 +139,10 @@ { addProperty(key, it.next()); } - return; - } - else - { - tokenAdd = new Vector(1); - tokenAdd.add(token); - } - - Object o = getPropertyDirect(key); - - if (o instanceof Container) - { - // There is already a container for our key in the config - // Simply add the new tokens - for (Iterator it = tokenAdd.iterator(); it.hasNext();) - { - ((Container) o).add(it.next()); - } } else { - // No Key at all or the token key is not a container. - Container c = new Container(); - - if (o != null) - { - // There is an element. Put it into the container - // at the first position - c.add(o); - } - - // Now gobble up the supplied objects - for (Iterator it = tokenAdd.iterator(); it.hasNext();) - { - c.add(it.next()); - } - - // Do we have a key? If not, we simply add either the container - // (If the element was a CSV) or the first element of the - // Container (if its size is 1) - - if (o == null && c.size() == 1) - { - // No Key existed and only one got put into the container. Then - // add the key direct. Do not mess with the container - addPropertyDirect(key, c.get(0)); - } - else - { - // Either a key already existed or there was a CSV supplied - // Add the Container to the Store - addPropertyDirect(key, c); - } + addPropertyDirect(key, token); } } @@ -1550,7 +1504,7 @@ * Private Wrapper class for Vector, so we can distinguish between * Vector objects and our container */ - class Container + static class Container { /** We're wrapping a List object (A vector) */ private List l = null; Index: src/java/org/apache/commons/configuration/BaseConfiguration.java =================================================================== RCS file: /home/cvspublic/jakarta-commons-sandbox/configuration/src/java/org/apache/commons/configuration/BaseConfiguration.java,v retrieving revision 1.15 diff -u -r1.15 BaseConfiguration.java --- src/java/org/apache/commons/configuration/BaseConfiguration.java 12 Oct 2003 09:32:30 -0000 1.15 +++ src/java/org/apache/commons/configuration/BaseConfiguration.java 14 Oct 2003 10:37:36 -0000 @@ -79,6 +79,7 @@ * @author <a href="mailto:[EMAIL PROTECTED]">Martin Poeschl</a> * @author <a href="mailto:[EMAIL PROTECTED]">Henning P. Schmiedehausen</a> * @author <a href="mailto:[EMAIL PROTECTED]">Konstantin Shaposhnikov</a> + * @author <a href="mailto:[EMAIL PROTECTED]">Oliver Heger</a> * * @version $Id: BaseConfiguration.java,v 1.15 2003/10/12 09:32:30 rdonkin Exp $ */ @@ -115,7 +116,39 @@ */ protected void addPropertyDirect(String key, Object obj) { - store.put(key, obj); + Object o = getPropertyDirect(key); + Object objAdd = null; + + if(o == null) + { + objAdd = obj; + } + else + { + if (o instanceof Container) + { + ((Container) o).add(obj); + } + else + { + // The token key is not a container. + Container c = new Container(); + + // There is an element. Put it into the container + // at the first position + c.add(o); + + // Now gobble up the supplied object + c.add(obj); + + objAdd = c; + } + } + + if(objAdd != null) + { + store.put(key, objAdd); + } } /**
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
