NightOwl888 commented on code in PR #814:
URL: https://github.com/apache/lucenenet/pull/814#discussion_r1161284290
##########
src/Lucene.Net/Store/ByteArrayDataOutput.cs:
##########
@@ -38,26 +39,51 @@ public class ByteArrayDataOutput : DataOutput
public ByteArrayDataOutput(byte[] bytes)
{
- Reset(bytes);
+ // LUCENENET: Changed to call private method to avoid virtual
method call in constructor
+ ResetInternal(bytes, 0, bytes.Length);
}
public ByteArrayDataOutput(byte[] bytes, int offset, int len)
{
- Reset(bytes, offset, len);
+ // LUCENENET: Changed to call private method to avoid virtual
method call in constructor
+ ResetInternal(bytes, offset, len);
}
public ByteArrayDataOutput()
{
- Reset(BytesRef.EMPTY_BYTES);
+ // LUCENENET: Changed to call private method to avoid virtual
method call in constructor
+ ResetInternal(BytesRef.EMPTY_BYTES, 0,
BytesRef.EMPTY_BYTES.Length);
}
- public virtual void Reset(byte[] bytes)
- {
- Reset(bytes, 0, bytes.Length);
- }
+ /// <summary>
+ ///
+ /// NOTE: When overriding this method, be aware that the constructor
of this class calls
+ /// a private method and not this virtual method. So if you need to
override
+ /// the behavior during the initialization, call your own private
method from the constructor
+ /// with whatever custom behavior you need.
+ /// </summary>
+ public virtual void Reset(byte[] bytes) =>
+ ResetInternal(bytes, 0, bytes.Length);
+
+ /// <summary>
+ ///
+ /// NOTE: When overriding this method, be aware that the constructor
of this class calls
+ /// a private method and not this virtual method. So if you need to
override
+ /// the behavior during the initialization, call your own private
method from the constructor
+ /// with whatever custom behavior you need.
+ /// </summary>
+ public virtual void Reset(byte[] bytes, int offset, int len) =>
+ ResetInternal(bytes, offset, len);
- public virtual void Reset(byte[] bytes, int offset, int len)
+ // LUCENENET specific - created a private method that can be called
+ // from the constructor and the Reset methods to avoid virtual method
+ // calls in the constructor.
+ private void ResetInternal(byte[] bytes, int offset, int len)
{
+ // LUCENENET: Added guard clause
+ if (bytes is null)
+ throw new ArgumentNullException(nameof(bytes));
+
Review Comment:
We should probably also add the [standard bounds checking guard
clauses](https://github.com/NightOwl888/J2N/blob/3d3c7211a39395c00aa3dfd1cdf9c01501391d87/src/J2N/Collections/Generic/List.SubList.cs#L185-L190).
Messages are
[here](https://github.com/NightOwl888/J2N/blob/3d3c7211a39395c00aa3dfd1cdf9c01501391d87/src/J2N/Resources/Strings.resx).
It would be great to rename these arguments `startIndex` and `length`, also,
but we should probably do a sweep of the codebase to get them all later rather
than doing them one class at a time.
##########
src/Lucene.Net/Store/ByteArrayDataOutput.cs:
##########
@@ -38,26 +39,51 @@ public class ByteArrayDataOutput : DataOutput
public ByteArrayDataOutput(byte[] bytes)
{
- Reset(bytes);
+ // LUCENENET: Changed to call private method to avoid virtual
method call in constructor
+ ResetInternal(bytes, 0, bytes.Length);
}
public ByteArrayDataOutput(byte[] bytes, int offset, int len)
{
- Reset(bytes, offset, len);
+ // LUCENENET: Changed to call private method to avoid virtual
method call in constructor
+ ResetInternal(bytes, offset, len);
}
public ByteArrayDataOutput()
{
- Reset(BytesRef.EMPTY_BYTES);
+ // LUCENENET: Changed to call private method to avoid virtual
method call in constructor
+ ResetInternal(BytesRef.EMPTY_BYTES, 0,
BytesRef.EMPTY_BYTES.Length);
}
- public virtual void Reset(byte[] bytes)
- {
- Reset(bytes, 0, bytes.Length);
- }
+ /// <summary>
+ ///
+ /// NOTE: When overriding this method, be aware that the constructor
of this class calls
+ /// a private method and not this virtual method. So if you need to
override
+ /// the behavior during the initialization, call your own private
method from the constructor
+ /// with whatever custom behavior you need.
+ /// </summary>
+ public virtual void Reset(byte[] bytes) =>
+ ResetInternal(bytes, 0, bytes.Length);
Review Comment:
We should either have a null guard clause here or alternatively, we can pass
`bytes?.Length ?? 0` as the last parameter.
--
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: dev-unsubscr...@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
