NightOwl888 commented on issue #1059: URL: https://github.com/apache/lucenenet/issues/1059#issuecomment-2543445154
> An update: I have the GroupingSearch class split out, and many of the interfaces and covariance removed, I just have one failing unit test that I am having difficulty tracking down. You can track my progress at my branch: https://github.com/paulirwin/lucene.net/tree/issue/1059 - or I welcome if anyone can pull that down and see what the problem is. > > The tests are a bit wonky as I have to create types that use `object` as the generic type parameter to mimic the previous covariance, since the tests randomly create either Term or Function versions based on a random boolean. Having an object-based wrapper class helps me get around needing covariance. The types BytesRef and MutableValue are already reference types, so converting them to object just for the tests is not a problem, it's just mildly annoying to have to cast them back out when needed. > > The GroupingSearch class now has three helper factory methods, but you also could use the split-out classes directly if you wanted to. I'll add XML doc comments before I publish the PR, for now I'm focused on getting it working. > > Once I get the failing test working, I'll finish removing the interfaces, then focus on converting IEnumerable back to ICollection and the other items on the list. I took a look and this seems to be a common problem that we have had when porting generic types from Java. C# sees `FirstPassGroupingCollector<BytesRef>` and `FirstPassGroupingCollector<MutableValue>` as 2 distinct types, but in Java both types can be stored in a variable of type `FirstPassGroupingCollector<?>`. The solution we commonly use is to create a non-generic abstraction for `FirstPassGroupingCollector` that the generic type implements/subclasses. ```c# public abstract class FirstPassGroupingCollector<T> : FirstPassGroupingCollector { // implementation } public abstract class FirstPassGroupingCollector // Note this could be an interface or an abstract class : ICollector { private protected FirstPassGroupingCollector() {} // Only allow internal use // implementation } ``` This would allow the type to be referenced without being aware of its generic type. ```c# private AbstractFirstPassGroupingCollector CreateFirstPassCollector<T>(string groupField, Sort groupSort, int topDocs, AbstractFirstPassGroupingCollector firstPassGroupingCollector) { // LUCENENET specific - we need to look for our wrapper types if (typeof(ObjectFirstPassGroupingCollector<BytesRef>).IsAssignableFrom(firstPassGroupingCollector.GetType())) { ValueSource vs = new BytesRefFieldSource(groupField); return new FunctionFirstPassGroupingCollector<MutableValue>(vs, new Hashtable(), groupSort, topDocs); } else if (typeof(ObjectFirstPassGroupingCollector<MutableValue>).IsAssignableFrom(firstPassGroupingCollector.GetType())) { return new TermFirstPassGroupingCollector(groupField, groupSort, topDocs); } fail(); return null; } ``` If you think about it, since we are having trouble testing it this way, it is very likely that users will need to be able to store these distinct types in a common variable type (after all, Java allows this), so making this a first class feature rather than a test mock is necessary. This would eliminate the need to create an object wrapper type. There may still be the need to cast somewhere, but there may be ways to mitigate that cast (perhaps by calling a method or property on a class that already is aware of the generic type and can handle the cast internally). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
