-
Notifications
You must be signed in to change notification settings - Fork 24
FLIP for Attachments #11
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
Conversation
turbolent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! It would be great to comment more on the differences to the extensions FLIP, and what the advantages and disadvantages this proposal has over it.
| pub let derivedX: Int | ||
|
|
||
| init (_ scalar: Int) { | ||
| self.derivedX = super.x * scalar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This initializer uses super - is this legal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea; the initializer is run when the attachment is added, so super is safe to use here.
joshuahannan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! I like it better than extensions too. I just had one question about multiple attachments of the same type
|
|
||
| ### Removing Attachments from a Type | ||
|
|
||
| Attachments can be removed with a new statement: `remove t from e`. The `t` value here is a type name, rather than a value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a composite type have multiple attachments of the same type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah this is a good point; implicitly it would have to be no since the attachments are referenced/looked-up by type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like that is going to be something that people will want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, in any situation where someone would want to have two different attachments of type T on a resource, they could also make a TManager type attachment that contains a [T] field, and achieve the same effect with a little indirection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my prediction is that most attachments will probably want to support multiple per resource, and I think it would just be better for the get attachments method to just return an array of attachments of the requested type, even if it is only a one-element array. I think requiring there be a wrapper type around multiple attachments of the same type just makes things more difficult, because then if you want to find a specific attachment for a resource, you can't just request its type any more. you also have to know the type of the wrapper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think I am perhaps not following; can you give me an example of where resources would want to support multiple copies of the same attachment?
Keep in mind that attachments cannot be removed from resources without destroying them; i.e. they are not objects in and of themselves. In this iteration of the proposal, for example, the Hat would not be an attachment for the Kitty because the Hat should be an independently valuable resource that can be bought and sold independently of the Kitty itself. Hence, the attachment for Kitty would be a HatManager that keeps track of all the Hats present on that Kitty, while the Hats are just normal resources.
This paradigm also allows for more flexibility in the implementation of Hats, since they can be written agnostically of the details of the Kitty, and the HatManager that actually gets attached to the Kitty just operates as an interface between them. In fact, under this model any resource X can be "attached" to any other resource Y using an XManager attachment for Y.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, that makes more sense
This FLIP is intended as an alternative to onflow/flow#1101, and should be considered in the context of the discussion on that FLIP, as well as in https://forum.onflow.org/t/extensibility/622 and onflow/cadence#357.
Please direct discussion on this FLIP to https://forum.onflow.org/t/flip-cadence-extensions-attachments/3645