lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adnan Duric <adu...@gmail.com>
Subject Re: FieldType refactoring?
Date Tue, 18 Oct 2011 22:02:45 GMT
Hi Mike,

Thanks for the response.

I was actually thinking about that as well. It seems to me that the
IndexableFieldType types only exist within the IndexableField types anyway.
We would just like the user to be able to specify a custom IFT but also have
a few of them available to the user and wrap them in friendly sounding
classes like TextField and StringField, etc...

If we have a FieldType behave as you suggested, namely something like:

IndexableFieldType ift = new FieldType(FieldType.INDEXED | FieldType.STORED
| ...);

then the user would be able to create a custom field type for their use.

IndexableField ifield = new Field("fieldname", text, ift);

I don't really think there are too many problems with this as it is, but
since (and correct me if I'm wrong) an IndexableFieldType only exists within
the context of an IndexableField, we can force the user to only be able to
instantiate an IndexableField through the Field concrete class:

IndexableField ifield = new Field("fieldname", text, new StoredFieldType());

where now StoredFieldType only creates FieldTypes:

final class StoredFieldType extends FieldType {
    public StoredFieldType() {
super(true, true, true...);
    }
}

final class UnstoredFieldType extends FieldType {
    public UnstoredFieldType() {
super(true, false, true...);
    }
}

This means that the IndexableField classes such as TextField, NumericField
and the like would not be needed at all, and would be replaced by FieldType
classes like TextType and NumericType for example; with only one constructor
instead of several. Also, at that point, Field must be declared final.


Best regards,


Adnan


On Tue, Oct 18, 2011 at 2:47 PM, Michael McCandless <
lucene@mikemccandless.com> wrote:

> Hi Adnan,
>
> I'm glad to hear you are using trunk; this is a good feedback!
>
> I agree about creating both StoredTextField and TextField (at the edge
> of the API): that's much easier to consume than what you now must do
> for the stored case (new Field("name", "value",
> TextField.STORED_TYPE).  I also really don't like how you don't know
> if StringField / TextField is stored by default.  Having XField and
> StoredXField would make that clear.
>
> So, regardless of how we create FTs/IFTs behind the scenes, I agree we
> should have separate sugar classes for StoredXField and XField.
>
> The expectation is the vast majority of users ("normal" users) will
> use the sugar field classes (Stored/StringField, Stored/TextField,
> NumericField, BinaryField, etc.) and never have to work with FieldType
> classes.
>
> Then, expert users are able to create arbitrary FieldType instances or
> way-expert can just make their own impls of IndexableFieldType (and
> IndexableField), ie bypassing Document/Field entirely.
>
> I agree it'd be nice to have immutable IFT instances, as you suggest,
> and then have no FieldType class at all.  The challenge is the API for
> creating such IFT instances (see the [long] discussion towards the end
> of LUCENE-2308)... basically the committers have strong disagreements
> on how to instantiate them (builder APIs vs
> ctor-taking-tons-of-boolean args APIs vs mutable + freeze).
>
> That said, I think the int bit flags approach (suggested on
> LUCENE-2308 but nobody has followed up on / worked out patch yet) is a
> good compromise, since nearly all of these settings are booleans.  So
> we could have something like:
>
>  new FieldType(INDEXED | STORED)
>
> So then FieldType would no longer be mutable, and it'd be a concrete
> IFT impl that uses bit flags under the hood to record the details for
> the type.
>
> Mike McCandless
>
> http://blog.mikemccandless.com
>
> On Tue, Oct 18, 2011 at 11:43 AM, Adnan Duric <aduric@gmail.com> wrote:
> > Hi guys,
> > First of all, this is a fantastic project. I'm having a great time
> playing
> > around with it.
> > I downloaded and compiled the trunk and I initially started trying to get
> > the examples working but quickly realized that the trunk version was
> > different than the previous versions. Anyway, long story short, I managed
> to
> > get it working.
> > However, I'd just like to comment on the functionality where the user
> adds
> > Field instances to a Document. Each Field has a name, value and a
> FieldType.
> > I got a little confused when I began to create a FieldType.
> > The FieldType class implements an IndexableFieldType, so it's effectively
> an
> > IndexableFieldType type but it also uses the same type to construct the
> > actual FieldType. This confused me because when I created my own
> FieldType,
> > I had to create a concrete implementation of IndexableFieldType and then
> > pass it to FieldType. This also means that there are two ways to create
> > IndexableFieldType types:
> > IndexableFieldType ift = new FieldType(new MyFieldType());
> > IndexableFieldType ift2 = new MyFieldType();
> > Then I noticed that FieldType just contains mutators to set the various
> > members of IndexableFieldType plus a toString() method. After seeing the
> > freeze() method, I began to realize that when a concrete FieldType is
> > created, some dependent objects have a chance to mutate the FieldType,
> and
> > then it should be immutable. These objects seem to be the various
> instances
> > of type IndexableField such as TextField, StringField, NumericField,
> etc...
> > Upon examination, I realized that when these classes are instantiated,
> they
> > create a FieldType and mutate some of its fields before freeze()'ing it.
> I
> > tried to see if I could somehow mutate after freezing but I couldn't find
> a
> > way to do it.
> > I instantiated a TextField and added to a Document:
> > IndexableField txtfield = new TextField(name, value);
> > doc.add(txtfield);
> > However, my test failed since the IndexableFieldType in txtfield was set
> to
> > TYPE_UNSTORED by default, and I guess I needed a TYPE_STORED. The only
> way
> > to have a "stored" version of TextField was to create a Field and set it
> to
> > a "stored" TextField like this:
> > doc.add(new Field("fieldname", text, TextField.TYPE_STORED));
> > The STORED versions for each class have to be created explicitly by going
> to
> > the Field directly. This got me thinking about how to consolidate the two
> > FieldTypes. Then I thought about getting rid of FieldType altogether
> since
> > it's just a mutator for IndexableFieldTypes, and having each concrete
> > IndexableField type (TextField, NumericField) constructed by directly
> > calling an IndexableFieldType. Since there seems to be "two" main
> > FieldTypes, maybe we could have a StoredFieldType and UnstoredFieldType
> that
> > implement IndexableFieldType:
> > final class StoredFieldType implements IndexableFieldType {
> >     public boolean indexed() { return true; }
> >     public boolean stored() { return true; }
> >     public boolean tokenized() { return true; }
> >
> > .
> > .
> > .
> >
> > }
> > final class UnstoredFieldType implements IndexableFieldType { ... }
> > And then have each IndexableField that is used, itself instantiate one of
> > the above IndexableFieldType. For example the TextField(String, String)
> > constructot:
> > /** Creates a new un-stored TextField */
> > public TextField(String name, String value) {
> >
> > super(name, value, new UnstoredFieldType());
> >
> > }
> > This way, the FieldType class is no longer needed, the IndexableField
> type
> > classes don't need to mutate anything, and, in the end, are instantiated
> the
> > same way. Of course, for the "stored" TextField, there needs to be a
> > StoredTextField class that instantiates a StoredFieldType, but I'm
> willing
> > to argue that it is better to have this duality appear closer to the
> 'edge'
> > of the API. Another advantage is that an entire hierarchy can be built
> with
> > IndexableFieldTypes by using a factory that creates them based on their
> > parameters.
> > Anyway, I'm probably missing some details and I'd appreciate some
> feedback.
> >
> > Thanks for your time and keep up the great work!
> >
> > Adnan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

Mime
View raw message