Feedback for how `CompareChain` handles `nil`

Chat & Discussions Discussions
2

To further advocate for this a bit I think it’s worth looking at how it handles more “normal” CompareChain scenarios since this != one is actually less common, and using the normal DateTime.compare? might actually be fine in this situation eg if new && old && DateTime.compare?(new, old) != :eq do

Consider:

if compare?(data.sync_cursor >= shadow.sync_cursor, DateTimeNilNegInfinity) do
  # stuff
end

What we are trying to deal with is that shadow.sync_cursor might be nil if the shadow is freshly initialized. In these cases we want nil to evaluate as less than any possible value of new_data.sync_cursor which is coming in. So in my proposal you’d have:

if compare?(data.sync_cursor >= shadow.sync_cursor || -inf(), DateTime) do
  # stuff
end

or

if compare?(data.sync_cursor >= -inf(shadow.sync_cursor), DateTime) do
  # stuff
end

to indicate that negative infinity is being used as a fallback for the value of shadow.sync_cursor.

This really improves the look I think of chained comparisons too:

compare?(si.activated_at <= geo_elem.latest_recorded_at <= si.deactivated_at || inf(), DateTime)

Because in this particular case we are being very explicit about which of the possible values we want to be OK with being nil.

2 Likes