[GitHub] [pulsar-dotpulsar] Chickenzilla commented on a change in pull request #50: Fix checking MessageId against null throwing NRE

[GitBox <gi...@apache.org>]

Chickenzilla commented on a change in pull request #50:
URL: https://github.com/apache/pulsar-dotpulsar/pull/50#discussion_r481288743



##########
File path: src/DotPulsar/MessageId.cs
##########
@@ -76,13 +76,13 @@ public override bool Equals(object o)
             => o is MessageId id && Equals(id);
 
         public bool Equals(MessageId other)
-            => LedgerId == other.LedgerId && EntryId == other.EntryId && Partition == other.Partition && BatchIndex == other.BatchIndex;
+            => !(other is null) && LedgerId == other.LedgerId && EntryId == other.EntryId && Partition == other.Partition && BatchIndex == other.BatchIndex;
 
         public static bool operator ==(MessageId x, MessageId y)
-            => x.Equals(y);
+            => object.ReferenceEquals(x, y) || (!object.ReferenceEquals(x, null) && x.Equals(y));

Review comment:
       Old school, and explicit intention: nothing is going to override that function and change its behavior.  I believe a modern compiler should know what I'm trying to do and optimize all of those into the same code anyhow.




----------------------------------------------------------------
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:
users@infra.apache.org