On 7/22/2012 8:36 AM, Jacopo Cappellato wrote:
On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
I agree there is a lot of feature envy between classes. The API could be
cleaned up a lot.
From my perspective, the GenericDispatcher.getLocalDispatcher method should
not exist - since it forces you to reference an implementation. Instead, there
should be a separate service dispatcher factory.
I agree this is the right direction to go.
In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext
and in the methods related to the creation/retrieval of
ServiceDispatcher/GenericDispatcher objects; this should simplify the
refactoring of the API and now the code is a bit cleaner and more readable.
Since this first pass is quite relevant in terms of code changes (unfortunately
I couldn't find a better way to split these in more commits) I would really
appreciate your reviews and bug reports (if any) and also your patience if you
will find issues caused by this change: I will do my best to fix them asap.
Some other things that could be included in a refactoring:
1. Move Temporal Expression code to the base component. Keep the
TemporalExpressionWorker class in the service component since it handles
expression persistence.
2. Move the model classes to a org.ofbiz.service.model package.
3. Make the model classes final and thread-safe.
-Adrian
