Re: Please do a partial revert: Re: svn commit: r802567 [1/5] - in /ofbiz/trunk: applications/order/src/org/ofbiz/order/shoppingcart/ applications/product/src/org/ofbiz/shipment/packing/ applications/product/src/org/ofbiz/shipment/verify/ applications/product/src/org/ofbiz/shipment/weigh...

[Tim Ruppert]

On Sep 8, 2009, at 1:36 PM, Adrian Crum wrote:

Adam Heath wrote:

David E Jones wrote:

On Sep 8, 2009, at 11:58 AM, Adam Heath wrote:

Adrian Crum wrote:

Adam Heath wrote:

Adrian Crum wrote:

Adam Heath wrote:

adri...@apache.org wrote:

Author: adrianc
Date: Sun Aug  9 18:04:26 2009
New Revision: 802567

URL: http://svn.apache.org/viewvc?rev=802567&view=rev
Log:
Refactored GenericDelegator:

1. Converted GenericDelegator to an interface. We already have
DelegatorInterface, but it isn't being used anywhere. Removed
DelegatorInterface.java.

2. Extracted the static, cached data from the GenericDelegator

GenericDelegator implementation holds a reference to the

instances of GenericDelegator.

3. Replaced the ThreadLocal variables with regular variables.




User modifications will have to replace
GenericDelegator.getGenericDelegator(...) with

code with the new setXxx methods, and remove the pop code. If
modifications used DelegatorInterface, replace that with
GenericDelegator.

Aside from those changes, this commit is backwards compatible.

implementation and put it in its own class - DelegatorData. The DelegatorData instance. This makes it possible to have per- thread ThreadLocal variables are no longer needed. Client code doesn't need to be concerned with pushing and popping the GenericDelegator state. This commit paves the way for the forthcoming ExecutionContext. DelegatorFactory.getGenericDelegator(...). Also, replace the push

No, it is not backwards compatible.


interface, then pre-compiled classes will *not* load.

Please, change GenericDelegator back to a class.


work to change the class to an interface.

I have external code(webslinger) that needs to support multiple

that impossible.  I have to have multiple versions of ofbiz

ofbiz version.

When a java class is compiled, the bytecode requests an interface named 'foo', or a class named 'foo'. If 'foo' changes from class to If DelegatorInterface wasn't used, and was just not uptodate with the method signatures, wouldn't the simpler thing have been to improve DelegatorInterface, then to change the class itself? It seems more versions of ofbiz(one all the way back to 512946). This change makes installed(pre/post this change), and compile the class once for each

Which is easier: rewrite all your Webslinger code to reference

existing code without making any changes?

DelegatorInterface instead of GenericDelegator, or just recompile your

That's just it, I wouldn't have to recompile *at all*, if
GenericDelegator stayed a class.  Nor would anyone else.

I don't have a problem with reverting it, but GenericDelegator will

become an interface eventually. If you take a look at David's
ExecutionContext branch, that is what he has planned.

Why? We already had DelegatorInterface, that has existed for years;

it was just never fleshed out.

There was a lot of discussion about this in the context of the

are in a single place, a low-level component that other framework
components will depend on, and then all code will use the framework

just be internal implementations of the framework interfaces and not
generally used directly).

reasons/goals behind the executioncontext branch(es). The basic idea is to move the framework to be accessible through a set of interfaces that through the interfaces instead of going directly to classes (which will For the rationale behind this, please see the write up I sent to this mailing list a few weeks ago about the ExecutionContext and related stuff.

Sure, understand all that. But this broke ABI. And I understand that

such a set of changes will *have* to break ABI.
If this change is to support the ExecutionContext branch, then it
should stay in that branch, 'til it is ready.

David and I are in agreement on the end result, and this point is where we disagree. Like you, David wants all changes to be made in the branch. I don't think a branch is needed. The interface extractions can be done a little at a time in the trunk. A branch with the kind of extensive changes that are planned will open up a HUGE can of worms when it is merged with the trunk. Consider that this one interface extraction resulted in three bug reports - what would happen if we introduced dozens of interface extractions in a single commit? Anyways, I don't mind reverting it. I'm just making the point that you will have to resolve these issues at some time in the future.

I have to admit that keeping everything outside and then merging things back is HUGE risk without proper unit tests on everything that it will touch. In this case - that would be everything in the system. I would rather it be incremental once the plan is laid down rather than hitting all at once.

-Adrian

smime.p7s
Description: S/MIME cryptographic signature