realm-swift icon indicating copy to clipboard operation
realm-swift copied to clipboard

[Breaking] Remove need for double optional / unwrapping with Map

Open leemaguire opened this issue 4 years ago • 5 comments
trafficstars

Currently if you want to access an object from a Map subscript you need to do:

map["foo"]??.myBar // some optional of optional value
// or
map["foo"]!!.myBar // doubly unwrapped value

With this PR we alleviate the need for the double unwrapping to make it feel more natural with a single unwrapping:

map["foo"]?.myBar // some optional value
// or
map["foo"]!.myBar // unwrapped value

This is a breaking change however.

leemaguire avatar Sep 09 '21 10:09 leemaguire

With this how would you distinguish a key not being present from the map containing nil? It also seems weird to make Object? have a different API from every other optional value type.

tgoyne avatar Sep 09 '21 15:09 tgoyne

With this how would you distinguish a key not being present from the map containing nil? It also seems weird to make Object? have a different API from every other optional value type.

I see what you mean, with this there is no way to distinguish Optional(nil). I had split the API because accessing an object from a map is a bit cumbersome. If this isn't a great addition, especially since its a breaking change I will just close this PR.

leemaguire avatar Sep 09 '21 21:09 leemaguire

The current API is indeed pretty cumbersome. Perhaps we could allow declaring Maps with non-optional Object types for when you don't care about distinguishing between object is nil and key isn't present? We'd report it to OS as still being nullable and then just do logic like this on access. The main awkwardness would be around .count and .keys. We'd have to pick one of the following:

  1. .count is O(N) and we check how many non-nil entries there are
  2. keys.count != .count (i.e. we filter nil values out of .keys and .values but not .count)
  3. Iterating over .keys and accessing each key in the Map can give nil.

tgoyne avatar Sep 09 '21 22:09 tgoyne

  1. .count is O(N) and we check how many non-nil entries there are

This sounds best, we could call get_pair() from OS and filter from there.

  1. keys.count != .count (i.e. we filter nil values out of .keys and .values but not .count)

I think this would lead to a lot of head scratching, as its not initially obvious why the counts don't match.

  1. Iterating over .keys and accessing each key in the Map can give nil.

This would be similar to the behaviour already in this PR, the nulled value key still exists, but on access you get nil.

leemaguire avatar Sep 10 '21 09:09 leemaguire

@tgoyne @leemaguire can we get movement on this?

jsflax avatar Feb 14 '22 17:02 jsflax