Comments inline.
Simon Nash wrote:
I think this convention is OK if we follow it everywhere, or make
it very clear where it applies and where it doesn't. For example,
extensions don't currently follow this pattern of putting
implementation code in an "impl" package. Would these be changed
or not?
That would be my recommendation, in general I think that our main/most
important extensions should follow our naming conventions. However I
wouldn't impose this on all extension developers.
If not, it may not be clear which unqualified top-level
packages are intended to contain SPIs and which are intended to
contain implementation code.
Whatever convention we decide to use, it's straightforward to
apply it to "pure interfaces" that are intended for extensions
to use directly, and to "pure implementations" that should not
be used by extensions. However, there's another category that
falls into a grey area. For example, BindingImpl is in the
org.apache.tuscany.assembly.impl package, but the new SPIs
require extension binding provider implementations to extend
this class directly.
Independent of the SPIs, I don't think it's a good idea to extend the
base BindingImpl or ImplementationImpl class directly. Doing this seems
to help, but effectively changes the apparent contract from Binding (a
clean interface) to BindingImpl, an implementation class that brings its
own specifics and will clutter the contract.
This raises two questions:
1. Should we be requiring extension writers to subclass core
implementation classes? I think this isn't a good idea.
I agree, but we actually don't require it. The CRUD implementation and
Echo binding samples don't extend the base assembly model classes for
example.
A cleaner approach would be for core implementation classes
that are intended as extension points to provide an explicit
extension mechanism other than subclassing. For example,
here's an "extension point" SPI that could be implemented
by extendible implementations like BindingImpl.
public interface ExtensionPoint {
public Object getExtensionPoint();
pubiic void setExtensionPoint(Object extensionPoint);
}
This would allow extension binding providers to depend only
on the Binding and ExtensionPoint interfaces, and not on the
BindingImpl class.
+1 from me. I think we should use more specific names, like
getBindingProvider/setBindingProvider, or createBindingProvider.
2. If we stay with the current approach, where do we put classes
like BindingImpl? I think they should be moved from "impl"
packages to "non-impl" SPI packages, to make it clear that
they are intended for use by extension code and are therefore
part of the SPI.
They shouldn't be extended. If we want to make sure that this doesn't
happen we can probably mark them final.
Simon
ant elder wrote:
On 5/4/07, Jean-Sebastien Delfino <[EMAIL PROTECTED]> wrote:
Simon Nash wrote:
> At the moment these interfaces are in the org.apache.tuscany.core
> package. This package name is also used by core implementation code,
> which is confusing.
>
> Is it the intention to change the package name for these SPI
interfaces
> to something else to avoid confusion between SPIs and implementation?
> For example, the SPIs could have a package name containing "spi".
>
> I think it's inmportant to do this in order to clearly separate SPI
> interfaces from implementation code.
>
> Simon
>
Good point, SPI interfaces and their implementations should be in
different packages.
I'm not too keen on adding .spi. to all the packages containing our
interfaces. I would prefer to have a simpler scheme:
o.a.t.core is the SPI
o.a.t.core.impl is the implementation
Same for our models:
o.a.t.contribution, o.a.t.assembly for the interfaces
o.a.t.contribution.impl , o.a.t.assembly.impl for the implementations
I see two benefits to that:
- the packages that most people use are simpler
- if you are using classes in .impl. you are warned that you're using
the implementations directly.
Sounds good to me.
...ant
--
Jean-Sebastien
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
