eerhardt commented on a change in pull request #10527:
URL: https://github.com/apache/arrow/pull/10527#discussion_r662433526



##########
File path: csharp/src/Apache.Arrow/Ipc/ArrowReaderImplementation.cs
##########
@@ -29,6 +29,13 @@ internal abstract class ArrowReaderImplementation : 
IDisposable
     {
         public Schema Schema { get; protected set; }
         protected bool HasReadSchema => Schema != null;
+        protected bool HasReadInitialDictionary { get; set; }
+        protected readonly DictionaryMemo _dictionaryMemo;
+
+        public ArrowReaderImplementation()
+        {
+            _dictionaryMemo = new DictionaryMemo();

Review comment:
       1. I wouldn't use create a new class for this, .NET has 
https://docs.microsoft.com/dotnet/api/system.lazy-1 already for it.
   2. Even better is to not use `Lazy<T>` at all, and just check for null 
before using it. You can make a private property to make this really easy:
   
   ```C#
   private DictionaryMemo _dictionaryMemo;
   
   private DictionaryMemo DictionaryMemo => _dictionaryMemo ??= new 
DictionaryMemo();
   
   // later in code that needs to use DictonaryMemo:
   
   DictionaryMemo.GetDictionaryType(id);
   ```
   
   This way, nothing new is allocated when we don't see a Dictionary batch in 
the payload.




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


Reply via email to