NightOwl888 commented on code in PR #809:
URL: https://github.com/apache/lucenenet/pull/809#discussion_r1161174100
##########
src/Lucene.Net/Util/RollingBuffer.cs:
##########
@@ -57,25 +58,17 @@ public abstract class RollingBuffer<T>
// How many valid Position are held in the
// array:
private int count;
-
- protected RollingBuffer()
- {
- for (var idx = 0; idx < buffer.Length; idx++)
- {
- buffer[idx] = NewInstance(); // TODO GIVE ROLLING BUFFER A
DELEGATE FOR NEW INSTANCE
- }
- }
+ private Func<T> factory;
protected RollingBuffer(Func<T> factory)
Review Comment:
I mocked up the above approach in a couple of different ways.
OptionA is more complicated to implement for end users because they have to
pass an extra generic type to both `RollingBuffer `and
`IRollingBufferItemFactory`. However, inside the factory there is no casting
required to see the state of the current buffer instance, as there is no cast
to see the state in Java.
OptionB is simpler to implement, but requires a cast from object to the
buffer type. Although it is guaranteed to be that type, using an `is` cast as
in my example makes it ambiguous what to do in the `else` case.
Some might consider OptionA to be overengineered. But given the rarity of
the case that end users will implement their own `RollingBuffer` I personally
think it is fine. And since it gives you a strong type for the buffer instance,
it eliminates casting or any confusion over what type we are receiving or
ambiguity when casting using `is`.
If we go with OptionA, there is an extra `IRollingBuffer` interface needed
to constrain us to a rolling buffer type without referring to any of its
generic closing types. In this case we probably can add the public members that
don't include `TItem` here instead of having an empty interface.
```c#
internal class Program
{
static void Main(string[] args)
{
var a = new OptionA.MyBuffer();
var b = new OptionB.MyBuffer();
Console.WriteLine("Hello World!");
}
}
namespace OptionA
{
public class Position
{
}
public abstract class RollingBuffer<TItem, TRollingBuffer> :
IRollingBuffer
where TRollingBuffer : IRollingBuffer
{
protected RollingBuffer(IRollingBufferItemFactory<TItem,
TRollingBuffer> itemFactory)
{
TItem[] buffer = new TItem[10];
for (int idx = 0; idx < buffer.Length; idx++)
{
// This intermediate cast to object is required because
of the generics, but it will never fail and is hidden from users.
buffer[idx] =
itemFactory.Create((TRollingBuffer)(object)this);
}
}
}
public interface IRollingBuffer // Interface required just so we can
"own" the rolling buffer without a generic closing type
{
}
public interface IRollingBufferItemFactory<out TItem, in
TRollingBuffer>
where TRollingBuffer : IRollingBuffer
{
TItem Create(TRollingBuffer rollingBuffer);
}
public class MyBuffer : RollingBuffer<Position, MyBuffer>
{
private object myState = new object();
public MyBuffer()
: base(MyPositionFactory.Default)
{
}
public class MyPositionFactory :
IRollingBufferItemFactory<Position, MyBuffer>
{
public static MyPositionFactory Default { get; } = new
MyPositionFactory();
public Position Create(MyBuffer rollingBuffer)
{
var state = rollingBuffer.myState; // Now we can see the
state
return new Position();
}
}
}
}
namespace OptionB
{
public class Position
{
}
public abstract class RollingBuffer<T>
{
protected RollingBuffer(IRollingBufferItemFactory<T> itemFactory)
{
T[] buffer = new T[10];
for (int idx = 0; idx < buffer.Length; idx++)
{
buffer[idx] = itemFactory.Create(this);
}
}
}
public interface IRollingBufferItemFactory<out T>
{
T Create(object rollingBuffer);
}
public class MyBuffer : RollingBuffer<Position>
{
private object myState = new object();
public MyBuffer()
: base(MyPositionFactory.Default)
{
}
public class MyPositionFactory :
IRollingBufferItemFactory<Position>
{
public static MyPositionFactory Default { get; } = new
MyPositionFactory();
public Position Create(object rollingBuffer)
{
if (rollingBuffer is MyBuffer buf)
{
var state = buf.myState; // Now we can see the state
}
// The else cannot technically happen, but if we return
in the above block, we would need a return or throw after this line, which
might not make any sense
return new Position();
}
}
}
}
```
--
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]
