I have opened JIRAs for the issues raised in this thread that need
further consideration or action. See comments inline below.
Simon
Simon Nash wrote:
Comments inline below.
Simon
Jean-Sebastien Delfino wrote:
[snip]
Simon Nash wrote:
1. BindingImpl is in org.apache.tuscany.assembly.impl but is
exposed as a subclassing extension point for binding extensions.
I removed BindingImpl as it is not a subclassing extension point. It
was there as a base class for SCABindingImpl, but several of us seem
to have taken it as a subclassing extension point, so it'll be cleaner
to remove it to avoid propagating that pattern.
Agreed, and I like the new implementation.
2. AbstractInvocationHandler and MessageImpl are in the
org.apache.tuscany.invocation interface package. The first
of these is intended for suclassing, so could be considered
an SPI, but the second is not.
I don't think that AbstractInvocationHandler should be an SPI, I
suggest to move to it to the core module. I am not sure why
Axis2CallbackInvocationHandler extends this class, as I don't really
understand the requirement to implement a Java Proxy Invocation
Handler in the Axis2 module. I'd suggest to remove it, but I may be
missing something...
However, MessageImpl is an SPI IMO, as various places in the runtime
including extensions should be able to new up a MessageImpl. Do people
think that it should be named differently? any proposal?
For implementation classes that implement SPI interfaces and need to
be newed up by extension code, a factory approach seems the right
solution. I'd be happy to make this change. Do we have any existing
similar factory pattern that I should follow?
I have opened TUSCANY-1278 for this. I found 6 places where a
MessageImpl is being newed up directly by code outside the core.
3. DefaultAssemblyFactory is in the interface package
org.apache.tuscany.assembly, but it extends from AssemblyFactoryImpl
which is in the implementation package
org.apache.tuscany.assembly.impl.
This means that "impl" code is showing up indirectly as part of an
SPI interface. The same arrangement is used for
DefaultPolicyFactory,
DefaultWSDLFactory, and DefaultWebServiceBindingFactory.
I don't find this shocking and I don't have a better (and reasonably
simple) idea to improve this. Do you have a proposal?
- move the XyzDefaultFactory to the impl package?
- get rid of XyzDefaultFactory and have the people new up a
XyzFactoryImpl directly?
- any other suggestion?
On further consideration, I think this pattern is OK because there's
no direct reference from extension code to any impl class. Other
arrangements are possible, such as:
package org.apache.tuscany.assembly;
import org.apache.tuscany.assembly.impl.AssemblyFactoryImpl;
public class DefaultAssemblyFactory {
private static AssemblyFactory defaultFactory;
public static AssemblyFactory getDefaultFactory() {
if (defaultFactory == null)
defaultFactory = new AssemblyFactoryImpl();
return defaultFactory;
}
}
I think this is slightly cleaner, but it's not a big deal one way
or the other.
I have opened TUSCANY-1280 for this. This change affects quite a
lot of files but I think it's worth doing. I'd appreciate feedback.
4. There's a PrintUtil class in the org.apache.tuscany.assembly.util
package. Why isn't this in the impl subpackage?
I didn't want to clutter the assembly.impl package with a util class
that has nothing to do with the assembly model implementation.
OK, that makes sense.
5. The org.apache.tuscany.assembly.xml package seems to contain
implementation classes that aren't in an "impl" subpackage.
I'm not sure that we want all non spi packages to be *.impl. In
general I think we should avoid extremes, like:
- have all spis in *.spi.* packages
- or have all non spi in *.impl packages
Looking at other Apache projects, many seem to follow a more
reasonable and balanced approach, which I hope we can do as well.
I replied to this on a separate thread.
6. The bindings don't seem to follow the "impl" subpackage convention,
except for binding-ws.
7. None of the core subpackages follow the "impl" subpackage
convention. They are called component, event, invocation, runtime,
store, util, and work.
We need to refactor these packages, these modules do not offer SPIs so
it's not necessarily very bad. I'd suggest the following:
- open JIRAs for this, to avoid losing these issues in the flow of emails
- evaluate their priority and stage this work, I'd suggest to do this
kind of refactoring after we have stabilized the function
- find volunteers to do this work :)
The core does offer SPIs. These are in a separate core-spi module in the
svn repository, but when we build a release distribution they will (I
presume)
be in the tuscany-all jar together with the core implementation code, and
not very clearly distinguished from it. For now I'll open a JIRA to track
the issue. I am willing to do this refactoring at a suitable time if we
can agree on a convention that everyone is OK with.
The JIRA for this refactoring is TUSCANY-1281.
8. I'm not sure whether the core-databinding module follows the
convention. There is no "impl" subpackage, but there seems to be
some implementation code.
9. core-spring nearly follows the convention, with everything in "impl"
subpackages except for one class ComponentContext in the "runtime"
subpackage.
ComponentContext is a temporary class and should go away when
SCADomain becomes available for use in that environment (which will
require a few changes in it).
OK, thanks.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
