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



##########
File path: csharp/src/Apache.Arrow/ArrowBuffer.Builder.cs
##########
@@ -22,51 +22,111 @@ namespace Apache.Arrow
 {
     public partial struct ArrowBuffer
     {
+        /// <summary>
+        /// The <see cref="Builder{T}"/> class is able to append value-type 
items, with fluent-style methods, to build
+        /// up an <see cref="ArrowBuffer"/> of contiguous items.
+        /// </summary>
+        /// <remarks>
+        /// Note that <see cref="bool"/> is not supported as a generic type 
argument for this class.  Please use
+        /// <see cref="BitPackedBuilder"/> instead.
+        /// </remarks>
+        /// <typeparam name="T">Value-type of item to build into a 
buffer.</typeparam>
         public class Builder<T>
             where T : struct
         {
             private const int DefaultCapacity = 8;
 
             private readonly int _size;
 
+            /// <summary>
+            /// Gets the number of items of current capacity.
+            /// </summary>
             public int Capacity => Memory.Length / _size;
+
+            /// <summary>
+            /// Gets the number of items currently appended.
+            /// </summary>
             public int Length { get; private set; }
+
+            /// <summary>
+            /// Gets the raw byte memory underpinning the builder.
+            /// </summary>
             public Memory<byte> Memory { get; private set; }
+
+            /// <summary>
+            /// Gets the span of memory underpinning the builder.
+            /// </summary>
             public Span<T> Span
             {
                 [MethodImpl(MethodImplOptions.AggressiveInlining)]
                 get => Memory.Span.CastTo<T>();
             }
 
+            /// <summary>
+            /// Creates an instance of the <see cref="Builder{T}"/> class.
+            /// </summary>
+            /// <param name="capacity">Number of items of initial capacity to 
reserve.</param>
             public Builder(int capacity = DefaultCapacity)
             {
+                // Using `bool` as the template argument, if used in an 
unrestricted fashion, would result in a buffer
+                // with inappropriate contents being produced.  Because C# 
does not support template specialisation,
+                // and because generic type constraints do not support 
negation, we will throw a runtime error to
+                // indicate that such a template type is not supported.
+                if (typeof(T) == typeof(bool))
+                {
+                    throw new ArgumentException(
+                        $"An instance of {nameof(Builder<T>)} cannot be 
instantiated, as `bool` is not an " +
+                        $"appropriate generic type to use with this class - 
please use {nameof(BitPackedBuilder)} " +
+                        $"instead");
+                }

Review comment:
       `NotSupportedException` is a better choice - I've made the change.




----------------------------------------------------------------
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:
[email protected]


Reply via email to