mr-smidge commented on a change in pull request #7654:
URL: https://github.com/apache/arrow/pull/7654#discussion_r456563263



##########
File path: csharp/src/Apache.Arrow/Arrays/DelegatingArrayBuilder.cs
##########
@@ -0,0 +1,96 @@
+//------------------------------------------------------------------------------
+// <copyright file="DelegatingArrayBuilder.cs" company="Jetstone Asset 
Management LLP">
+// Copyright (C) Jetstone Asset Management LLP.  All rights reserved.
+// Unauthorised copying of this file, via any medium, is strictly prohibited.
+// Proprietary and confidential
+// </copyright>
+// <author>Adam Szmigin</author>
+//------------------------------------------------------------------------------
+
+using System;
+using Apache.Arrow.Memory;
+
+namespace Apache.Arrow
+{
+    /// <summary>
+    /// The <see cref="DelegatingArrayBuilder{T,TArray,TBuilder}"/> class can 
be used as the base for any array builder
+    /// that needs to delegate most of its functionality to an inner array 
builder.
+    /// </summary>
+    /// <remarks>
+    /// The typical use case is when an array builder may accept a number of 
different types as input, but which are
+    /// all internally converted to a single type for assembly into an array.
+    /// </remarks>
+    /// <typeparam name="T">Type of item accepted by inner array 
builder.</typeparam>
+    /// <typeparam name="TArray">Type of array produced by this (and the 
inner) builder.</typeparam>
+    /// <typeparam name="TBuilder">Type of builder (see Curiously-Recurring 
Template Pattern).</typeparam>
+    public abstract class DelegatingArrayBuilder<T, TArray, TBuilder> : 
IArrowArrayBuilder<TArray, TBuilder>

Review comment:
       If I were to refactor `TimestampArray` today, I would use 
`DelegatingArrayBuilder` as the builder's base class, adding methods to accept 
`DateTimeOffset` (and probably `NodaTime.Instant` if we use NodaTime for the 
TZDB).
   
   I don't mind which direction we take for this PR, but have a minor 
preference to keep `DelegatingArrayBuilder` (because we know that we will need 
to refactor `TimestampArray` in order for the C# codebase to become 
Production-ready).
   
   I've raised https://issues.apache.org/jira/browse/ARROW-9515 for the 
enhancements to `TimestampArray` in future.

##########
File path: csharp/src/Apache.Arrow/Arrays/DelegatingArrayBuilder.cs
##########
@@ -0,0 +1,96 @@
+//------------------------------------------------------------------------------
+// <copyright file="DelegatingArrayBuilder.cs" company="Jetstone Asset 
Management LLP">
+// Copyright (C) Jetstone Asset Management LLP.  All rights reserved.
+// Unauthorised copying of this file, via any medium, is strictly prohibited.
+// Proprietary and confidential
+// </copyright>
+// <author>Adam Szmigin</author>
+//------------------------------------------------------------------------------
+
+using System;
+using Apache.Arrow.Memory;
+
+namespace Apache.Arrow
+{
+    /// <summary>
+    /// The <see cref="DelegatingArrayBuilder{T,TArray,TBuilder}"/> class can 
be used as the base for any array builder
+    /// that needs to delegate most of its functionality to an inner array 
builder.
+    /// </summary>
+    /// <remarks>
+    /// The typical use case is when an array builder may accept a number of 
different types as input, but which are
+    /// all internally converted to a single type for assembly into an array.
+    /// </remarks>
+    /// <typeparam name="T">Type of item accepted by inner array 
builder.</typeparam>
+    /// <typeparam name="TArray">Type of array produced by this (and the 
inner) builder.</typeparam>
+    /// <typeparam name="TBuilder">Type of builder (see Curiously-Recurring 
Template Pattern).</typeparam>
+    public abstract class DelegatingArrayBuilder<T, TArray, TBuilder> : 
IArrowArrayBuilder<TArray, TBuilder>

Review comment:
       If I were to refactor `TimestampArray` today, I would use 
`DelegatingArrayBuilder` as the builder's base class, adding methods to accept 
`DateTimeOffset` (and probably `NodaTime.Instant` if we use NodaTime for the 
TZDB).
   
   I don't mind which direction we take for this PR, but have a preference to 
keep `DelegatingArrayBuilder` (because we know that we will need to refactor 
`TimestampArray` in order for the C# codebase to become Production-ready).
   
   I've raised https://issues.apache.org/jira/browse/ARROW-9515 for the 
enhancements to `TimestampArray` in future.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to