You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Oliver Heger <ol...@oliver-heger.de> on 2008/02/17 20:49:52 UTC
Re: svn commit: r628395 - in /commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2:
beanutils/BeanHelper.java interpol/ConfigurationInterpolator.java
This commit contains a lot of noise related to formatting changes. This
makes it very difficult to find out what you actually changed.
Can you please try to minimize reformatting as much as possible?
Thanks
Oliver
ebourg@apache.org schrieb:
> Author: ebourg
> Date: Sat Feb 16 16:20:15 2008
> New Revision: 628395
>
> URL: http://svn.apache.org/viewvc?rev=628395&view=rev
> Log:
> More generification
>
> Modified:
> commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/beanutils/BeanHelper.java
> commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java
>
> Modified: commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/beanutils/BeanHelper.java
> URL: http://svn.apache.org/viewvc/commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/beanutils/BeanHelper.java?rev=628395&r1=628394&r2=628395&view=diff
> ==============================================================================
> --- commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/beanutils/BeanHelper.java (original)
> +++ commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/beanutils/BeanHelper.java Sat Feb 16 16:20:15 2008
> @@ -56,7 +56,7 @@
> public class BeanHelper
> {
> /** Stores a map with the registered bean factories. */
> - private static Map beanFactories = Collections.synchronizedMap(new HashMap());
> + private static Map<String, BeanFactory> beanFactories = Collections.synchronizedMap(new HashMap<String, BeanFactory>());
>
> /**
> * Stores the default bean factory, which will be used if no other factory
> @@ -84,8 +84,7 @@
> {
> if (name == null)
> {
> - throw new IllegalArgumentException(
> - "Name for bean factory must not be null!");
> + throw new IllegalArgumentException("Name for bean factory must not be null!");
> }
> if (factory == null)
> {
> @@ -139,8 +138,7 @@
> {
> if (factory == null)
> {
> - throw new IllegalArgumentException(
> - "Default bean factory must not be null!");
> + throw new IllegalArgumentException("Default bean factory must not be null!");
> }
> defaultBeanFactory = factory;
> }
> @@ -155,8 +153,7 @@
> * @param data the bean declaration
> * @throws ConfigurationRuntimeException if a property cannot be set
> */
> - public static void initBean(Object bean, BeanDeclaration data)
> - throws ConfigurationRuntimeException
> + public static void initBean(Object bean, BeanDeclaration data) throws ConfigurationRuntimeException
> {
> Map properties = data.getBeanProperties();
> if (properties != null)
> @@ -174,8 +171,7 @@
> for (Iterator it = nestedBeans.keySet().iterator(); it.hasNext();)
> {
> String propName = (String) it.next();
> - initProperty(bean, propName, createBean(
> - (BeanDeclaration) nestedBeans.get(propName), null));
> + initProperty(bean, propName, createBean((BeanDeclaration) nestedBeans.get(propName), null));
> }
> }
> }
> @@ -194,8 +190,7 @@
> {
> if (!PropertyUtils.isWriteable(bean, propName))
> {
> - throw new ConfigurationRuntimeException("Property " + propName
> - + " cannot be set!");
> + throw new ConfigurationRuntimeException("Property " + propName + " cannot be set!");
> }
>
> try
> @@ -235,15 +230,13 @@
> {
> if (data == null)
> {
> - throw new IllegalArgumentException(
> - "Bean declaration must not be null!");
> + throw new IllegalArgumentException("Bean declaration must not be null!");
> }
>
> BeanFactory factory = fetchBeanFactory(data);
> try
> {
> - return factory.createBean(fetchBeanClass(data, defaultClass,
> - factory), data, param);
> + return factory.createBean(fetchBeanClass(data, defaultClass, factory), data, param);
> }
> catch (Exception ex)
> {
> @@ -292,8 +285,7 @@
> * @return the class object for the specified name
> * @throws ClassNotFoundException if the class cannot be loaded
> */
> - static Class loadClass(String name, Class callingClass)
> - throws ClassNotFoundException
> + static Class loadClass(String name, Class callingClass) throws ClassNotFoundException
> {
> return ClassUtils.getClass(name);
> }
> @@ -311,8 +303,7 @@
> * @return the class of the bean to be created
> * @throws ConfigurationRuntimeException if the class cannot be determined
> */
> - private static Class fetchBeanClass(BeanDeclaration data,
> - Class defaultClass, BeanFactory factory)
> + private static Class fetchBeanClass(BeanDeclaration data, Class defaultClass, BeanFactory factory)
> throws ConfigurationRuntimeException
> {
> String clsName = data.getBeanClassName();
> @@ -351,8 +342,7 @@
> * @return the bean factory to use
> * @throws ConfigurationRuntimeException if the factory cannot be determined
> */
> - private static BeanFactory fetchBeanFactory(BeanDeclaration data)
> - throws ConfigurationRuntimeException
> + private static BeanFactory fetchBeanFactory(BeanDeclaration data) throws ConfigurationRuntimeException
> {
> String factoryName = data.getBeanFactoryName();
> if (factoryName != null)
> @@ -360,8 +350,7 @@
> BeanFactory factory = (BeanFactory) beanFactories.get(factoryName);
> if (factory == null)
> {
> - throw new ConfigurationRuntimeException(
> - "Unknown bean factory: " + factoryName);
> + throw new ConfigurationRuntimeException("Unknown bean factory: " + factoryName);
> }
> else
> {
>
> Modified: commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java
> URL: http://svn.apache.org/viewvc/commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java?rev=628395&r1=628394&r2=628395&view=diff
> ==============================================================================
> --- commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java (original)
> +++ commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java Sat Feb 16 16:20:15 2008
> @@ -120,10 +120,10 @@
> private static final char PREFIX_SEPARATOR = ':';
>
> /** A map with the globally registered lookup objects. */
> - private static Map globalLookups;
> + private static Map<String, StrLookup> globalLookups;
>
> /** A map with the locally registered lookup objects. */
> - private Map localLookups;
> + private Map<String, StrLookup> localLookups;
>
> /** Stores the default lookup object. */
> private StrLookup defaultLookup;
> @@ -135,7 +135,7 @@
> {
> synchronized (globalLookups)
> {
> - localLookups = new HashMap(globalLookups);
> + localLookups = new HashMap<String, StrLookup>(globalLookups);
> }
> }
>
> @@ -155,13 +155,11 @@
> {
> if (prefix == null)
> {
> - throw new IllegalArgumentException(
> - "Prefix for lookup object must not be null!");
> + throw new IllegalArgumentException("Prefix for lookup object must not be null!");
> }
> if (lookup == null)
> {
> - throw new IllegalArgumentException(
> - "Lookup object must not be null!");
> + throw new IllegalArgumentException("Lookup object must not be null!");
> }
> synchronized (globalLookups)
> {
> @@ -326,7 +324,7 @@
> // static initializer, sets up the map with the standard lookups
> static
> {
> - globalLookups = new HashMap();
> + globalLookups = new HashMap<String, StrLookup>();
> globalLookups.put(PREFIX_SYSPROPERTIES, StrLookup.systemPropertiesLookup());
> globalLookups.put(PREFIX_CONSTANTS, new ConstantLookup());
> }
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r628395 - in /commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2:
beanutils/BeanHelper.java interpol/ConfigurationInterpolator.java
Posted by Oliver Heger <ol...@oliver-heger.de>.
sebb schrieb:
> On 17/02/2008, Emmanuel Bourg <eb...@apache.org> wrote:
>> Oliver Heger a écrit :
>>> This commit contains a lot of noise related to formatting changes. This
>>> makes it very difficult to find out what you actually changed.
>>>
>>> Can you please try to minimize reformatting as much as possible?
>>>
>>> Thanks
>>> Oliver
>> Do you mind if we set the column limit to 120 characters ? 80 characters
>> is quite small with today screens and wrapped code is difficult to read.
>>
>
> Even with a largish screen, 120 characters is quite a lot if using an
> IDE with multiple windows.
>
> Whether wrapped code is difficult to read depends on where the code is
> wrapped ...
Ah, the old discussion about line lengths ;-)
I personally prefer shorter lengths (maybe up to 100 characters), but
would not veto against longer lines. However I am against reformatting
the existing code base.
Oliver
>
>> Emmanuel Bourg
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r628395 - in /commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2:
beanutils/BeanHelper.java interpol/ConfigurationInterpolator.java
Posted by Thorbjørn Ravn Andersen <th...@gmail.com>.
Emmanuel Bourg skrev den 17-02-2008 23:36:
>
> it's less annoying after a comma :
>
> foo.doSomething(param1,
> param2,
> param2);
I have found it convenient to introduce explanatory variables, named
according to the parameter they are to be assigned (which should also be
named well).
For instance this method
public static void send(String smtpHost, int smtpPort,
String from, String to,
String subject, String content)
could be called with
send("mailhost.com", 25, "me", "you", "hello world", "lots of text");
which may break given real life constants. By extracting the parameters
into variables you can explain each one. By reusing the parameter names
you copy the signature of the method which may make it easier to
remember the purpose. Also this usually requires less line breaks than
having the parameter expression in the function calls. For modern
JVM's the performance is identical (as the variables will be inlined).
smtpHost = "mailhost.com";
smtpPort = 25;
from = "me";
to = "you";
subject = "hello world";
content = "....";
send(smtpHost, smtpPort, from, to, subject, content)
This works especially well for booleans where you generally never can
remember what ", true, true)" versus ", true, false)" means.
--
Thorbjørn
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r628395 - in /commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2:
beanutils/BeanHelper.java interpol/ConfigurationInterpolator.java
Posted by Emmanuel Bourg <eb...@apache.org>.
sebb a écrit :
> Whether wrapped code is difficult to read depends on where the code is
> wrapped ...
The kind of wrapping that I find difficult to read is a wrapping on a
method invocation, for example:
foo
.bar()
or wrapping just after a parenthesis :
Foo foo = new Foo(
bar);
it's less annoying after a comma :
foo.doSomething(param1,
param2,
param2);
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r628395 - in /commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2: beanutils/BeanHelper.java interpol/ConfigurationInterpolator.java
Posted by sebb <se...@gmail.com>.
On 17/02/2008, Emmanuel Bourg <eb...@apache.org> wrote:
> Oliver Heger a écrit :
> > This commit contains a lot of noise related to formatting changes. This
> > makes it very difficult to find out what you actually changed.
> >
> > Can you please try to minimize reformatting as much as possible?
> >
> > Thanks
> > Oliver
>
> Do you mind if we set the column limit to 120 characters ? 80 characters
> is quite small with today screens and wrapped code is difficult to read.
>
Even with a largish screen, 120 characters is quite a lot if using an
IDE with multiple windows.
Whether wrapped code is difficult to read depends on where the code is
wrapped ...
> Emmanuel Bourg
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r628395 - in /commons/proper/configuration/branches/configuration2_experimental/src/main/java/org/apache/commons/configuration2:
beanutils/BeanHelper.java interpol/ConfigurationInterpolator.java
Posted by Emmanuel Bourg <eb...@apache.org>.
Oliver Heger a écrit :
> This commit contains a lot of noise related to formatting changes. This
> makes it very difficult to find out what you actually changed.
>
> Can you please try to minimize reformatting as much as possible?
>
> Thanks
> Oliver
Do you mind if we set the column limit to 120 characters ? 80 characters
is quite small with today screens and wrapped code is difficult to read.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org