Unit test for IndexerFactory: IndexerFactoryTest

[ge0ffrey]

I think it would be good to test the IndexFactory in isolation by calling buildUniKeysExtractor().

Specifically, this piece of code looks wierd to me.

            for (var i = 1; i <= joinerCount; i++) {
                var joinerType = i < joinerCount ? joiner.getJoinerType(i) : null;
                var previousJoinerType = joiner.getJoinerType(i - 1);
                if (joinerType != JoinerType.EQUAL || previousJoinerType != joinerType) {
                    /*
                     * Equal joiner is building a composite key with preceding equal joiner(s).
                     * Does not apply to joiners other than equal; for those, each indexer has its own simple key.
                     */
                    joinerTypeMap.put(i, previousJoinerType);
                }
            }

What does it do given (equals, equals, equals, lessThan, equals, equals)?
If the equals are merged, why would it have 5 entries in the map instead of 2? // not true, it it only has 3 entries, which makes sense.
Why are keys in the map not zero based (see put(i, previousJoinerType))?

[triceo]

The code works exactly as you suggest.
The if makes sure to only add an entry when the joiner type changes - so for the example you gave, equals + equals + equals + lessThan + equals + equals becomes equals + lessThan + equals.

I agree that the unit test should be better. That said, this code is tested in production every day; if it didn't work, we'd know by now.

[ge0ffrey]

this code is tested in production every day; if it didn't work, we'd know by now.

Unless it's an edge case. (Who in their right mind does equals + equals + equals + lessThan + equals + equals?)
Or it's correct, but it creates unnecesary overhead.

It's probably ok - I can see how it works now - but let's add a unit test to confirm?