You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2009/09/29 19:06:58 UTC
Re: svn commit: r819067 [1/3] - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/
entity/src/META-INF/services/ entity/src/org/ofbiz/entity/
adrianc@apache.org wrote:
> Author: adrianc
> Date: Fri Sep 25 23:36:13 2009
> New Revision: 819067
>
> URL: http://svn.apache.org/viewvc?rev=819067&view=rev
> Log:
> Refactored GenericDelegator.java based on a design proposed by Adam Heath on the dev mailing list.
>
> DelegatorInterface.java has been replaced by Delegator.java.
>
>
> Added:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java (with props)
> ofbiz/trunk/framework/entity/src/META-INF/services/org.ofbiz.entity.DelegatorFactory (with props)
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java (with props)
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java (with props)
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java (with props)
> Removed:
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorInterface.java
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
>
> Added: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java?rev=819067&view=auto
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java (added)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java Fri Sep 25 23:36:13 2009
> @@ -0,0 +1,32 @@
> +package org.ofbiz.base.util;
> +
> +/** Factory interface. */
> +public interface Factory<T> {
public interface Factory<T, O>
> +
> + /** Returns an instance of <code>T</code>.
> + *
> + * @param obj Optional object to be used in <code>T</code>'s construction,
> + * or to be used as a selector key
> + * @return An instance of <code>T</code>
> + */
> + public T getInstance(Object obj);
public T getInstance(O obj);
> +
> +}
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java?rev=819067&r1=819066&r2=819067&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java Fri Sep 25 23:36:13 2009
> @@ -23,6 +23,8 @@
> import java.io.IOException;
> import java.io.ObjectOutputStream;
> import java.io.InputStream;
> +import java.util.Iterator;
> +import javax.imageio.spi.ServiceRegistry;
>
> /**
> * UtilObject
> @@ -171,4 +173,17 @@
> if (o1 == null) return 0;
> return o1.hashCode();
> }
> +
> + @SuppressWarnings("unchecked")
> + public static <T> T getObjectFromFactory(Class<T> factoryInterface, Object obj) throws ClassNotFoundException {
public static <T, O> getObjectFromFactory(Class<Factory<T, O>>
factoryInterface, O obj), then remove the cast below, and then
@SuppressWarnings.
> + Iterator<Factory<T>> it = (Iterator<Factory<T>>) ServiceRegistry.lookupProviders(factoryInterface);
> + while (it.hasNext()) {
> + Factory<T> factory = it.next();
> + T instance = factory.getInstance(obj);
> + if (instance != null) {
> + return instance;
> + }
> + }
> + throw new ClassNotFoundException(factoryInterface.getClass().getName());
> + }
> }
>
> Added: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java?rev=819067&view=auto
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java (added)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java Fri Sep 25 23:36:13 2009
> +public interface Delegator {
> + /**
> + * Finds all Generic entities NOTE 20080502: 14 references; all changed to
> + * findList
> + *
> + * @param entityName
> + * The Name of the Entity as defined in the entity XML file
> + * @return List containing all Generic entities
> + * @deprecated Use findList() instead
> + */
> + @Deprecated
> + public List<GenericValue> findAll(String entityName) throws GenericEntityException;
Don't add @Deprecated methods to *new* interfaces or classes. That
makes no sense.
Also, interfaces don't need the public keyword attached to methods;
it's superflous.
Re: svn commit: r819067 [1/3] - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/
entity/src/META-INF/services/ entity/src/org/ofbiz/entity/
Posted by Adam Heath <do...@brainfood.com>.
Adrian Crum wrote:
> I understand what you're suggesting. But if the intention is to replace
>
> GenericDelegator delegator = GenericDelegator.getDelegator(delegatorName);
>
> with
>
> Delegator delegator = (Delegator)
> UtilObject.getObjectFromFactory(DelegatorFactory.class, delegatorName);
>
> then the Delegator interface will need to support the same methods as
> GenericDelegator.
Nope. Leave GenericDelegator.getDelegator, which returns
GenericDelegator. This way, all existing code continues to work, no
changes at all. Then, existing scripts can be modified one at a time.
Base helper methods should be modified first, so that they take a
Delegator instead of a GenericDelegator. This will just require a
recompile(as calling code will still be using a GenericDelegator,
which implements Delegator).
Basically, find all methods that have an incoming GenericDelegator
object, but do *not* forward it on to any other method(be careful
about services that call out). Modify those methods first. Then move
slowly up the chain.
Re: svn commit: r819067 [1/3] - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/
entity/src/META-INF/services/ entity/src/org/ofbiz/entity/
Posted by Adrian Crum <ad...@hlmksw.com>.
I understand what you're suggesting. But if the intention is to replace
GenericDelegator delegator = GenericDelegator.getDelegator(delegatorName);
with
Delegator delegator = (Delegator)
UtilObject.getObjectFromFactory(DelegatorFactory.class, delegatorName);
then the Delegator interface will need to support the same methods as
GenericDelegator.
-Adrian
Adam Heath wrote:
> Adrian Crum wrote:
>> Adam,
>>
>> Thanks for the tips!
>>
>> The public modifier and @Deprecated in the Delegator interface are
>> carried over from GenericDelegator.java. I used Eclipse to extract the
>> interface - hence the public modifier. David Jones deprecated the
>> methods - not me. We can remove the deprecated methods when they are no
>> longer used.
>
> Nono.
>
> You've added a *new* interface. Adding immediately deprecated methods
> to a *new* interface or class is stupid.
>
> I'm not suggesting removing the methods from GenericDelegator. Just
> the definitions from the interface.
>
Re: svn commit: r819067 [1/3] - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/
entity/src/META-INF/services/ entity/src/org/ofbiz/entity/
Posted by Adam Heath <do...@brainfood.com>.
Adrian Crum wrote:
> Adam,
>
> Thanks for the tips!
>
> The public modifier and @Deprecated in the Delegator interface are
> carried over from GenericDelegator.java. I used Eclipse to extract the
> interface - hence the public modifier. David Jones deprecated the
> methods - not me. We can remove the deprecated methods when they are no
> longer used.
Nono.
You've added a *new* interface. Adding immediately deprecated methods
to a *new* interface or class is stupid.
I'm not suggesting removing the methods from GenericDelegator. Just
the definitions from the interface.
Re: svn commit: r819067 [1/3] - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/
entity/src/META-INF/services/ entity/src/org/ofbiz/entity/
Posted by Adrian Crum <ad...@hlmksw.com>.
Adam,
Thanks for the tips!
The public modifier and @Deprecated in the Delegator interface are
carried over from GenericDelegator.java. I used Eclipse to extract the
interface - hence the public modifier. David Jones deprecated the
methods - not me. We can remove the deprecated methods when they are no
longer used.
-Adrian
Adam Heath wrote:
> adrianc@apache.org wrote:
>> Author: adrianc
>> Date: Fri Sep 25 23:36:13 2009
>> New Revision: 819067
>>
>> URL: http://svn.apache.org/viewvc?rev=819067&view=rev
>> Log:
>> Refactored GenericDelegator.java based on a design proposed by Adam Heath on the dev mailing list.
>>
>> DelegatorInterface.java has been replaced by Delegator.java.
>>
>>
>> Added:
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java (with props)
>> ofbiz/trunk/framework/entity/src/META-INF/services/org.ofbiz.entity.DelegatorFactory (with props)
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java (with props)
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactory.java (with props)
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorFactoryImpl.java (with props)
>> Removed:
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/DelegatorInterface.java
>> Modified:
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
>>
>> Added: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java?rev=819067&view=auto
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java (added)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/Factory.java Fri Sep 25 23:36:13 2009
>> @@ -0,0 +1,32 @@
>> +package org.ofbiz.base.util;
>> +
>> +/** Factory interface. */
>> +public interface Factory<T> {
>
> public interface Factory<T, O>
>
>> +
>> + /** Returns an instance of <code>T</code>.
>> + *
>> + * @param obj Optional object to be used in <code>T</code>'s construction,
>> + * or to be used as a selector key
>> + * @return An instance of <code>T</code>
>> + */
>> + public T getInstance(Object obj);
>
> public T getInstance(O obj);
>
>> +
>> +}
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java?rev=819067&r1=819066&r2=819067&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilObject.java Fri Sep 25 23:36:13 2009
>> @@ -23,6 +23,8 @@
>> import java.io.IOException;
>> import java.io.ObjectOutputStream;
>> import java.io.InputStream;
>> +import java.util.Iterator;
>> +import javax.imageio.spi.ServiceRegistry;
>>
>> /**
>> * UtilObject
>> @@ -171,4 +173,17 @@
>> if (o1 == null) return 0;
>> return o1.hashCode();
>> }
>> +
>> + @SuppressWarnings("unchecked")
>> + public static <T> T getObjectFromFactory(Class<T> factoryInterface, Object obj) throws ClassNotFoundException {
>
> public static <T, O> getObjectFromFactory(Class<Factory<T, O>>
> factoryInterface, O obj), then remove the cast below, and then
> @SuppressWarnings.
>
>> + Iterator<Factory<T>> it = (Iterator<Factory<T>>) ServiceRegistry.lookupProviders(factoryInterface);
>> + while (it.hasNext()) {
>> + Factory<T> factory = it.next();
>> + T instance = factory.getInstance(obj);
>> + if (instance != null) {
>> + return instance;
>> + }
>> + }
>> + throw new ClassNotFoundException(factoryInterface.getClass().getName());
>> + }
>> }
>>
>> Added: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java?rev=819067&view=auto
>> ==============================================================================
>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java (added)
>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/Delegator.java Fri Sep 25 23:36:13 2009
>> +public interface Delegator {
>> + /**
>> + * Finds all Generic entities NOTE 20080502: 14 references; all changed to
>> + * findList
>> + *
>> + * @param entityName
>> + * The Name of the Entity as defined in the entity XML file
>> + * @return List containing all Generic entities
>> + * @deprecated Use findList() instead
>> + */
>> + @Deprecated
>> + public List<GenericValue> findAll(String entityName) throws GenericEntityException;
>
> Don't add @Deprecated methods to *new* interfaces or classes. That
> makes no sense.
>
> Also, interfaces don't need the public keyword attached to methods;
> it's superflous.
>
>