Skip to main content

[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

Back to the top