lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adnan Duric <adu...@gmail.com>
Subject FieldType refactoring?
Date Tue, 18 Oct 2011 15:43:18 GMT
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

Mime
View raw message