Proper Notification object comparison

[cmer]

Comparing two objects that evaluate to the same byte representation should be 'true'. Particularly useful in tests/specs.

[vanstee]

Instead of rescueing would something like this work?

other.respond_to?(:to_bytes) &&
  self.to_bytes == other.to_bytes

[alindeman]

👍

[cmer]

No because to_bytes can throw an exception if the payload is too large. That's why I did it this way.

But I see what you mean. Perhaps we should actually do both.

[vanstee]

Oh yeah good call. That's got me thinking now. What if you want to check equality of 2 invalid notifications? Comparing instance variables might be better than the result of to_bytes. Thoughts?

[stevenharman]

Can we limit this to only rescuing Grocer::PayloadTooLargeError?

[cmer]

Added type comparison. Would love to see this merged so I can remove my local monkey patch.

[stevenharman]

if the Class is the same, they should both respond_to?(:to_bytes), no?

[cmer]

If they're different, they might still both respond_to?(:to_bytes), which is why I make the check

[stevenharman]

What I'm saying is, you could simplify this to just be

def ==(other)
  self.class === other &&
  self.to_bytes == other.to_bytes
end

[stevenharman]

comparing instance variables might be better than the result of to_bytes.

Agreed. Since to_bytes can raise when invalid, rendering any two invalid notifications NOT equal, even if they are... maybe we should be comparing the important state? For example, comparing their encoded_payload should suffice, no?

[vanstee]

Yeah I agree that serializing the data is not important for equality.

[stevenharman]

Oh wait... there is more to the state than just the payload... like the :identifier, :device_token, etc. Hmm...