Patch.

good catch. But why do you throw NPE? the rest of the setXYZ methods respect null by setting the default. I don't like that setMP is not consistent with the rest of the setters.

Good point. I copied the behavior of setCodec which throws a NPE although you are right that most methods seem to set the default value...

I think that setCodec should also be modified then, to something like that:

if (codec == null) {
    codec = Codec.getDefault();
    if (codec == null) {
      throw new NullPointerException();
    }
}

Just like the ctor does. And the NPE should include some message too (both in the ctor and setter).

I'm not sure I like the fact that passing null to setXXX actually sets the default value, what do other committers think?

it seems more likely that someone would pass null due to a program mistake, rather than
wanting the defaults.

My point is that if someone wants to use the default value, all he has to do is to never call the setter? Moreover users can't pass null to methods that expect primitive types (such as setMaxBufferedDocs) so throwing an exception when encountering null would be more consistent?

We need to be consistent across the setters. We throw IllegalArg in the other setters (which take primitives), so maybe throw that and not NPE?

We throw IllegalArg in the other setters (which take primitives), so maybe throw that and not NPE?

+1 I'll update the patch.

Updated patch. IndexWriterConfig.setXXX methods now throw an IllegalArgumentException when passed null instead of setting the default value. Tests pass.

[trunk commit] Adrien Grand
http://svn.apache.org/viewvc?view=revision&revision=1456564

LUCENE-4833: Make IndexWriterConfig setters throw an exception when passed null if null is not a valid value.

[branch_4x commit] Adrien Grand
http://svn.apache.org/viewvc?view=revision&revision=1456567

LUCENE-4833: Make IndexWriterConfig setters throw an exception when passed null if null is not a valid value (merged from r1456564).