You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As alluded to in #117, a FullTextIndexBuilder doesn't really follow the best practice for builders which is for each action to return a clone of the previous instance with the changes applied.
At the moment this won't do what some might expect:
varcoreBuilder=newFullTextIndexBuilder<int>();varindex1=coreBuilder.WithIndexModificationAction(async(idx)=>// Do something).Build();var index2 = coreBuilder.WithIndexModificationAction(async(idx)=>// Do something).Build();
Because the coreBuilder instance will end up with two index modification actions when index2 is built.
Either:
Builder instances need to be immutable
An exception should be thrown if Build is called multiple times
An exception should be thrown if the builder is mutated after it has been built (this would at least allow for multiple calls of Builds)
Something else I haven't thought of yet.
The text was updated successfully, but these errors were encountered:
FullTextIndexBuilder doesn't really follow the best practice for builders which is for each action to return a clone of the previous instance with the changes applied.
I didn't know how to express it that well, but that was my unthinking assumption. But I'm not sure whether returning a modified clone is even a well-established pattern outside the functional world. In non-functional .Net land, I have come across libraries with fluent configuration APIs to modify an instance - so such behaviour is to be expected.
I guess what tripped me up was that reusing the same builder instance worked just fine - until I introduced saving each index using the .WithIndexModificationActionlike in your example without creating new builder instances (unlike your example).
Thanks @h0lg - I think it just feels like bad API design if you're not guided down the best path, even if it's by virtue of an exception thrown at runtime.
As alluded to in #117, a
FullTextIndexBuilder
doesn't really follow the best practice for builders which is for each action to return a clone of the previous instance with the changes applied.At the moment this won't do what some might expect:
Because the
coreBuilder
instance will end up with two index modification actions whenindex2
is built.Either:
The text was updated successfully, but these errors were encountered: