[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
Re: [Wtp-wst-dev] comments about new API design in psychopath engine
|
Hi Mukul and others
On Tue, Apr 19, 2011 at 12:58 PM, Mukul Gandhi
<mukul.gandhi@xxxxxxxxxx> wrote:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=320958
The PsychoPath XPath engine's new API
is now available on WTP Source Editing HEAD location. I got a chance now
to look at this new API, and here are few comments please:
<questions for Jesper/>
You've created new interfaces like TypeDefinition,
PrimitiveType and so on. In fact you seem to have created a new native
PsychoPath type system in package org.eclipse.wst.xml.xpath2.api.typesystem.
This is almost conceptually identical to Xerces's type system implementation.
I've few comments about this,
1. I'm very skeptical of this effort
(at least rebranding of the schema type system). You've almost identically
forked Xerces's type system and wrapped it (taking in and taking out the
objects) in another nomenclature. Why don't we stick with Xerces's type
system framework (it's so much time tested).
To my opinion, the new type system framework
introduces unknown risks to the implementation (and an unnecessary learning
curve to people like me).
The reason for introducing this type system interface for XPath2 is solely to support other type system implementations than Xerces, since this project is used for more than Xerces.
Specifically, we can now use the type information from the SSE DOMs to make type-specific queries against an open WTP editor, since it can implement the type APIs using the information from the CMNode information. Other processors, like Xalan, might have a different type system to plug in. This is why the interfaces where produced. Xerces' type interfaces are not cleanly separated from its implementation and was thus not a viable choice for this.
2. There's a minor comment about the
package naming convention you've chosen for the new API system. You use
package name like, org.eclipse.wst.xml.xpath2.api. This is quite uncommon
convention. An API class or method should be just available in some package,
with public access. The package name should have domain meaning (.api introduces
a technical terminology to the API implementation, which to my opinion
is not a good choice).
While I agree that the name is not ideal, I specifically asked for your review in early March, but got no objections?
Do you intend to keep this terminology
permanently, or you're thinking to change it sometime in future?
I was very much hoping to not have to change this going forward, provided the XPath2 should still follow the yearly release-train schedules of WTP. Such a change should have happened last milestone, really, or at least before end of business today, due to M7 cool-down next week. If you have a better suggestion, please speak up. One idea is to use org.eclipse.wst.xml.xpath2.typesystem and org.eclipse.wst.xml.xpath2.typesystem, and leave the implementations in org.eclipse.wst.xml.xpath2.processor.
We could move all of this into
org.eclipse.wst.xml.xpath2.processor if we delete all the old interfaces and leave no backwards compatibility (and since Xerces-beta is the only concern here, your can have your way...)
I hope you've noticed all the improvements in the API -- untying StaticContext from DynamicContext, function libraries are not re-entrant, many fewer objects are allocated, etc. -- I've made those especially for embedded use of the library such as Xerces'.
-Jesper