On 27 Aug 2009, at 13:06, Emmanuel Bernard wrote:


On 27 août 09, at 13:07, Manik Surtani wrote:

Very elegant.  I'm generally a big fan of 'builder' patterns like this, but this really isn't a DSL, is it?  :)  When you first mentioned a DSL I had visions of defining a new grammar and an ANTLR parser, etc.  But that is overkill.

This is called an internal DSL, ie you use Java as the language, not some external representation.


This approach certainly works, and will almost certainly perform better too.  One question: for the sake of brevity, why SealedQueryBuilder instead of QueryBuilder ?  :)

The name is not right yet

There are two things:
- the query builder that lets you define the analyzer
- the query builder that has an analyzer assigned and lets you build query
What name is best for each of them.

I thought this stuff you mentioned made sense:

queryBuilder.withAnalyzer(Analyzer)
queryBuilder.withEntityAnalyzer(Class<?>)
queryBuilder.basedOnEntityAnalyzer(Class<?>)
                    .overridesForField(String field, Analyzer)
                    .overridesForField(String field, Analyzer)
                    .build() //sucky name

Perhaps rename the static factory methods to something like:

QueryBuilder.getQueryBuilder(Analyzer)
QueryBuilder.getQueryBuilder(Class<?>)

and QueryBuilder instances have overrideAnalyzerForField(String, Analyzer).  Why do you need the build() method at the end?




Also, I still think that if this is a generic helper factory that helps you build Lucene queries - and has no knowledge of how and where the query is used (why should it?) - then this should be something people can use outside of HS or Infinispan.  E.g., directly with Lucene.

As of today this code is technically pure Lucene but to be honest the idea of passing an analyzer multiplexer (like the one we receive from searchFactory.getAnalyzer<Class<?>)) is not wildly spread in Lucene and potentially cumbersome wo the declarative approach of HSearch.

The second problem is that some potential improvements will require inner knowledge of HSearch:
- object parameters (and not string params) do require to know the FieldBridge of the property. This is a pure HSearch notion.
- "property literal" like JPA is introducing could be added to replace the String-based field approach in some situations. Though I don't think that it would be a perfect fit.
- spell checker (the old idea we had)

That been said, if the API ends up being pure Lucene and once we stabilize it, we can contribute it back even though I am not necessarily a huge fan of ASL.

Not it doesn't have to be either ASL or even hosted at Apache.  I guess what I am suggesting is perhaps even a separate project - LuceneQueryBuilder or something  - which plain-old-Lucene users could use as well.  Doesn't matter where it's hosted or what the license is - as long as its ASL or LGPL :)

--
Manik Surtani
Lead, Infinispan
Lead, JBoss Cache