Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add opt-in protocol CasePathSubscriptable to add similar functionality to struct keyPath subscripts #1

Closed
wants to merge 4 commits into from

Conversation

cdmcmahon
Copy link

Hey there! Love this library. One thing kept eating at me and it was exemplified by these lines from the README:

user[keyPath: \User.id] // 42
(/Authentication.authenticated).extract(from: authentication) // Optional("cafebeef")

These feel asymmetrical, and I don't think they need to be. With a protocol extension we can define a subscript that looks almost identical to structs.

user[keyPath: \User.id] // 42
authentication[casePath: /Authentication.authenticated] // Optional("cafebeef")

Voila! It's also opt in so, like the operators in the library, completely ignorable if so desired.

One reservation I have is that someone could apply this protocol to something other than an enum, which would have unexpected and undefined behaviors. However, I think this is fundamentally similar to the limitations of the library in general and just the limitations of implementing this at the 3rd party library level.

@cdmcmahon cdmcmahon changed the title Add opt-in protocol CasePathSubscriptable to add similar functionality to struct key path subscripts Add opt-in protocol CasePathSubscriptable to add similar functionality to struct keyPath subscripts Feb 5, 2020
@stephencelis
Copy link
Member

stephencelis commented Feb 5, 2020

Thanks for the PR! This was definitely something we thought about and I'd love to know your opinion once we walk through the conclusions we came to.

I have the same feelings as you and would love symmetry. I think it would feel nice to have a language-level ability (without marker protocols) to do this:

result[casePath: /Result.success] // Optional(42)

And it's nice that we can recover this with a protocol!

What feels strange, though, is when we try to apply the same functionality to setters:

result[casePath: /Result.success] = 1337

There are 2 peculiar things about the above statement:

  1. It needs an existing root result to create a new root enum (while CasePath can embed a value in a root without needing an existing root to do so).
  2. I think for the types to line up the above needs to be able to assign optionally, which allows for result[casePath: /Result.success] = nil, which is kinda weird!

So while subscript semantics makes total sense for key paths, we're just not sure they make sense for case paths, and even though the getter in this PR makes half of the story a lot more symmetrical, we just don't know how to carry the other half of the story along the way ☹️ This might just be a limitation of how subscripts work in Swift right now.

I'd like to point out that @gringoireDM's EnumKit offers a nice protocol-oriented solution for this kind of enum associated value extraction (he even uses a subscript!). We're just not quite sure what to do about the setter/embedder side of things.

On the brighter side, those key path subscripts are generally only used by library creators. End-users just need to pass concrete key paths (or case paths) to the APIs those libraries provide. So even though we might not be able to create a symmetric API for library creators, the library consumers don't have to know 😅

What are your thoughts?

@cdmcmahon
Copy link
Author

cdmcmahon commented Feb 5, 2020

First off, thank you for the response! I really appreciate it. And yes, I assumed you guys had thought of this, so it's very validating of my thought process to hear so. 🤗

I have some thoughts about the setter side of the subscript idea, both that come from my experience in other languages (primarily Java at work 😑) and also just generally 😄. To be honest, when starting to write this PR description, I began to include a second "reservation" regarding the lack setter, but then talked myself out of it being a negative for the following reasons.

Regarding the two specific peculiarities mentioned, I'll start in reverse order with your second point: that because the subscript signature is subscript <T>(casePath casePath: CasePath<Self, T>) -> T?, the related setter would need to accept an optional. This would be problematic in the most basic cases:

enum Foo {
  case one(Int)
  case two(String)
}

Both cases state that they have an honest Int/String associated value, which means that allowing a setter of an optional could get a value of Foo in a strange (and in my mind, invalid) state. Imagine the following (which may not even compile, I didn't test it):

let example: Foo = .one(2)
example[casePath: /Foo.one] = nil
example[casePath: /Foo.one] // returns nil

At first glance, this vaguely works because the case path subscript setter does accept an optional. Furthermore, the case path subscript getter does return an optional and nil is a valid return value for such.

However, it seems wrong because one expects the nil response on a case path getter to indicate that example is not of the case Foo.one. Because of this, allowing assignment of null seemed undesirable. It's basically a case of allowing an invalid representation as the .one case requires an Int, but we can set it as nil

Which brings me to the first peculiarity brought up, that the case path subscript setter requires an existing value of the enum on which to set the new case and associated values, using the embed function of the case path under the hood. This only affects mutable variables, which are rare with enums, but they are also unpredictable to the average developer, especially given most experience with structs. Consider the following example:

var example2: Foo = .one(2)
example2[casePath: /Foo.two] = "bar"

With experience in structs, one might expect the value example2 has two properties, /Foo.one with the 2 and Foo.two with the value "bar". However, in reality, this example entirely overwrites the former value with the latter. Either way this brings me to my reasoning about not needing a setter on the case path subscript, which is that the above is completely writeable without a setter subscript, simply using the natural enum constructors swift gives us:

// completely equivalent to the above example
var example2: Foo = .one(2)
example2 = .two("bar")

tl;dr: So ultimately, I believe that allowing a setter subscript would 1) allow invalid data to be represented by allowing non-nil associated values to be set as nil (again, which might not even compile, I didn't even try it) and 2) would duplicate functionality of the initializers of enums. As such, defining a get-only subscript seems to be the perfect solution.

This gets to what I see as interesting property of sum types vs. product types. Setting a property on product types can change one field without need to consider any other field of the type, because by definition, that new value will "fit" will all the values of all other properties. However, changing a field on a sum type either:

  • changes the sum type value's case, and thus requires new values for all associated values of the enum, or
  • maintains the sum type value's case, which still essentially requires values for all associated values within the case, even if some/all of them are the same

In both of those cases, it needs to provide new values that cover all of the associated values for the case. Thus, for sum types, initialization and setting are not that fundamentally different. This is why many languages require enums to be immutable (Java 👀).

(That tl;dr was itself pretty long, but hopefully I've gotten my main reasoning across 😄)

@cdmcmahon
Copy link
Author

Oh, one last comment is that if you decided to include this protocol, it could be renamed to CasePathGetSubscriptable/CasePathSubscriptGettable/anything else to indicate that the subscript is get-only, but it might be getting pretty verbose... 😂

@gringoireDM
Copy link

gringoireDM commented Feb 5, 2020

On EnumKit that uses this sort of protocols with subscript for getter and setter I faced the same issue, but ultimately I decided that for the time being it is not a vital issue to solve, by ignoring all instances of nil, except if the associated value itself is nillable, so that auth[case: Authentication.authenticated] = nil results in a no op.

When working with collections and combine extensions for this application on enum (they are part of EnumKit) and later on the RxSwift extension in RxEnumKit, I've noticed that this is the best choice to avoid issues. You can't stop users from passing nil in a setter (especially when it is a result of a stream of optional values) but as api writer I can avoid this from unwanted side effects.

We should not forget that the symmetry with struct is wanted, but there are objective differences to take into account. It's important to note that while for properties we have keyPaths, for enum this library proposes CasePaths AKA "Do something just if the case matches".

KeyPaths:

  • Give me the value of property X
  • Set the value to Z of property X

CasePaths:

  • Give me the associated value if the case matches Y
  • Set the associated value to Z if the case matches Y

There is an intrinsic condition in CasePaths that does not exist in KeyPaths. It's not nice to have nil setters, but for now swift doesn't allow us to force a different type on set.

@stephencelis
Copy link
Member

@cdmcmahon I think we totally agree that the setter is not a great idea, and I think you covered just why that is rather thoroughly 😄

This is exactly why we weren't quite ready to add this subscript, though, because it feels a bit imbalanced to offer the getter but handle the "setter" in a different way. Part of what you described can be seen in the semantics of optional-chaining, where readability is optional but writability is non-optional:

user?.name = "Blob" // ✅
user?.name = nil // 🛑

This kind of getter/setter pair is not currently representable in a property/subscript in Swift, though hopefully in the future it will be!

I think we'd like to be cautious in adding this functionality for now, but we'd like to revisit it in the future. Or maybe Swift will get first-class case paths and we can retire this repo 😂

@gringoireDM We made the same nil-swallowing decision in the properties generated by swift-enum-properties. It was pragmatic, but never felt quite right. Hopefully the language offers solutions in the future, though!

@cdmcmahon
Copy link
Author

This is exactly why we weren't quite ready to add this subscript, though, because it feels a bit imbalanced to offer the getter but handle the "setter" in a different way. Part of what you described can be seen in the semantics of optional-chaining, where readability is optional but writability is non-optional

[...]

This kind of getter/setter pair is not currently representable in a property/subscript in Swift, though hopefully in the future it will be!

Maybe this part of my point didn't come through as clearly, but I don't know if a setter like that should be representable. To illustrate this, let's imagine it did.

enum Foo {
  case one(Int)
  case two(String, Int)
}

var example2: Foo = .one(2)
example2[casePath: /Foo.one] = nil // Step A: 🛑 
example2[casePath: /Foo.one] = 3 // Step B: ✅ 
example2[casePath: /Foo.two] = ("bar", 4) // Step C: ✅ 
// Only want to change one associated value? Still have to provide both
example2[casePath: /Foo.two] = ("bar", 5) // Step D: ✅ 

Given that such a setter would require all the associated values for the case you want to set even when only changing some of them (see Step D), all it does is replicate the initializer. You can also think through the implementation wise. Such a setter would just call casePath.embed. And what is embed? One of the static enum case initializers!

(Another note is that this could be confusing given peoples expectations from keyPath subscript setters. Step B loses the original associated value of 2. That said, one assumes someone using this lib would be aware of enum/struct distinctions.)

I fully could be missing something or some case, but because of this a get only subscript seems even more correct than the hypothetical getter & setter above. It reflects an actual distinction between structs and enums, as far as I can tell.

Alas, I'll leave it be for now. Thank you for your consideration and help!

@cdmcmahon cdmcmahon closed this Feb 5, 2020
@stephencelis
Copy link
Member

Maybe this part of my point didn't come through as clearly, but I don't know if a setter like that should be representable. To illustrate this, let's imagine it did.

For enums I think we agree. But for optionally-chained paths it would still be useful to have these semantics:

var foo: Int? {
  get {  }
  nonoptional set { ... }
}

This would allow for things like writable optionally-chained key paths, which currently isn't possible, but should be!

Still not quite sure exactly what case paths should look like if they lived in the language as first-class citizens, but we'll continue to think on it. Thanks for the example and discussion!

@gringoireDM
Copy link

@gringoireDM We made the same nil-swallowing decision in the properties generated by swift-enum-properties. It was pragmatic, but never felt quite right. Hopefully the language offers solutions in the future, though!

Yeah, it's the least "evil" thing to do. Not many options there unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants