Copilot commented on code in PR #48:
URL: https://github.com/apache/activemq-nms-amqp/pull/48#discussion_r2180420311


##########
src/NMS.AMQP/Provider/Amqp/Message/AmqpNmsBytesMessageFacade.cs:
##########
@@ -45,58 +46,60 @@ public BinaryReader GetDataReader()
 
             if (byteIn == null)
             {
-                Data body = GetBinaryFromBody();
-                Stream dataStream = new MemoryStream(body.Binary, false);
+                byte[] body = Content;
+                Stream dataStream = new MemoryStream(body, false);
                 byteIn = new EndianBinaryReader(dataStream);
             }
 
             return byteIn;
         }
 
-        private Data GetBinaryFromBody()
+        public byte[] Content
         {
-            RestrictedDescribed body = Message.BodySection;
-            Data result = EMPTY_DATA;
-            if (body == null)
+            get
             {
-                return result;
-            }
-            else if (body is Data)
-            {
-                byte[] binary = (body as Data).Binary;
-                if (binary != null && binary.Length != 0)
+                RestrictedDescribed body = Message.BodySection;
+                byte[] result = EMPTY_BINARY;
+                if (body == null)
                 {
-                    return body as Data;
+                    return result;
                 }
-            }
-            else if (body is AmqpValue)
-            {
-                object value = (body as AmqpValue).Value;
-                if (value == null)
+                else if (body is Data)
                 {
-                    return result;
+                    byte[] binary = (body as Data).Binary;
+                    if (binary != null && binary.Length != 0)
+                    {
+                        return binary;
+                    }
                 }
-
-                if (value is byte[])
+                else if (body is AmqpValue)
                 {
-                    byte[] dataValue = value as byte[];
-                    if (dataValue.Length > 0)
+                    object value = (body as AmqpValue).Value;
+                    if (value == null)
                     {
-                        result = new Data();
-                        result.Binary = dataValue;
+                        return result;
+                    }
+                    if (value is byte[])
+                    {
+                        byte[] dataValue = value as byte[];
+                        if (dataValue.Length > 0)
+                        {
+                            return dataValue;
+                        }
+                    }
+                    else
+                    {
+                        throw new IllegalStateException("Unexpected Amqp value 
content-type: " + value.GetType().FullName);
                     }
                 }
                 else
                 {
-                    throw new IllegalStateException("Unexpected Amqp value 
content-type: " + value.GetType().FullName);
+                    throw new IllegalStateException("Unexpected body 
content-type: " + body.GetType().FullName);
                 }
-            }
-            else
-            {
-                throw new IllegalStateException("Unexpected body content-type: 
" + body.GetType().FullName);
-            }
 
-            return result;
+                return result;
+            }
+            set => Message.BodySection = new Data { Binary = value };

Review Comment:
   After updating the body section, reset any cached readers/writers (`byteIn`, 
`byteOut`) to avoid stale stream state when consuming or writing content again.
   ```suggestion
               set
               {
                   Message.BodySection = new Data { Binary = value };
                   byteIn = null;
                   byteOut = null;
               }
   ```



##########
src/NMS.AMQP/Provider/Amqp/Message/AmqpNmsBytesMessageFacade.cs:
##########
@@ -31,10 +31,11 @@ public class AmqpNmsBytesMessageFacade : 
AmqpNmsMessageFacade, INmsBytesMessageF
         private EndianBinaryReader byteIn = null;
         private EndianBinaryWriter byteOut = null;
 
-        private static readonly Data EMPTY_DATA = new Data { Binary = new 
byte[0] };
+        private static readonly byte[] EMPTY_BINARY = new byte[0];
+        private static readonly Data EMPTY_DATA = new Data { Binary = 
EMPTY_BINARY };
 

Review Comment:
   The EMPTY_DATA constant is no longer referenced after the refactoring. 
Consider removing it to clean up unused code.
   ```suggestion
   
   ```



##########
src/NMS.AMQP/Message/NmsBytesMessage.cs:
##########
@@ -292,15 +295,19 @@ public int ReadBytes(byte[] value)
         public int ReadBytes(byte[] value, int length)
         {
             InitializeReading();
+            return ReadBytes(dataIn, value, length);
+        }
 
+        private int ReadBytes(BinaryReader binaryReader, byte[] value, int 
length)

Review Comment:
   [nitpick] This private overload shares the same name as the public 
`ReadBytes` method, which may cause confusion. Consider renaming it to 
something like `ReadBytesInternal`.
   ```suggestion
               return ReadBytesInternal(dataIn, value, length);
           }
   
           private int ReadBytesInternal(BinaryReader binaryReader, byte[] 
value, int length)
   ```



-- 
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: gitbox-unsubscr...@activemq.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to