-
Notifications
You must be signed in to change notification settings - Fork 715
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
Suggest using getters in mixins #4767
Conversation
I've long understood how mixins could provide value, but it took me way to long to understand how to access state in the super class (which makes mixins far more powerful). I'm not sure if this rewording is enough or if it may be helpful to add a simple example of a getter or accessing a member of the superclass in the mixin. I like the brief docs, but this is a case where one more example would've gone a long way for me.
For example, the mixin might depend on being able to invoke a method | ||
that the mixin doesn't define. | ||
Sometimes the mixin might depend on being able to invoke a method or access fields | ||
that the mixin doesn't define. To require classes that use the mixin to define these, |
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.
that the mixin doesn't define. To require classes that use the mixin to define these, | |
that the mixin doesn't define. Mixins can't define a constructor, | |
so they can't use constructor parameters to instantiate their own fields. | |
To require classes that use the mixin to define these, |
I don't know that this is important to include in the docs, but this is where I was getting stuck with the existing example from the docs.
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.
@KyleFin: Updated the copy, would like a tech review to be sure.
Hi @KyleFin, thanks for working on this and apologies for the delay. I just looked around for an example of using a getter in a mixin, or accessing members of the superclass at all, and couldn't find anything in our docs, so I think adding that example is a good point. In the mean time, I wonder if you could read through this comment from a different-but-related PR, and if the example snippets or the subsequent conversation about the paragraph you edited provide any extra insight for you? I am planning to restructure the entirety of the mixins page based on that comment thread, and I think your point here is important to factor in. I'm just wondering if you have any thoughts on how your change fits into that conversation. |
that the mixin doesn't define. | ||
Sometimes the mixin might depend on being able to invoke a method or access fields | ||
that the mixin doesn't define. To require classes that use the mixin to define these, | ||
you can define getters in the mixin or restrict the types that can use a mixin. |
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.
What is meant by "restrict the types that can use a mixin"?
I'm curious how a mixin restricts types that use it, and feel this point should probably be expanded somewhere.
PTAL @munificent
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'm not Bob 🙃 but I think that wording puts the emphasis on the wrong part of the point its trying to make. It's the wording that is currently on the page, it's not new to this PR:
Sometimes you might want to restrict the types that can use a mixin.
The point though, isn't restricting the types that can use it, but ensuring that any methods the mixin might rely on that are defined by another type are in fact actually defined before the mixin can be used. I.e. that the mixin can only be used with the on
directive specifying the superclass that defines whatever it is the mixin relies on (or the abstract
way to accomplish the same thing, explained in this comment and (kind of) in the new section I added after this PR was created).
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.
The initial PR comment says:
it took me way too long to understand how to access state in the super class (which makes mixins far more powerful).
But I think this is a (very common) misunderstanding of the inheritance order that mixins give you. What you most often want is to be able to access state in the mixin's subclass. For example:
/// Can be applied to any type with a [name] property and provides an
/// implementation of [hashCode] and operator `==` in terms of it.
mixin NameIdentity {
String get name;
int get hashCode => name.hashCode;
bool operator ==(other) => other is NameIdentity && name == other.name;
}
class Person with NameIdentity {
final String name;
Person(this.name);
}
I think this is probably the kind of code that @KyleFin has in mind. (If not, correct me. :) ). In this example, Person
is the subclass of NameIdentity
.
So there's no need for an on
clause on NameIdentity
. (The rule for on
clauses is simple: If your mixin doesn't have any super
calls, you don't need an on
clause.) Instead, all you need is an abstract member declaration. In this case, it's String get name;
.
I agree with @KyleFin that users often don't realize they can have mixins that access state on the class they are applied to by calling getters which are defined as abstract on the mixin. I like the idea of adding a little emphasis about that to the docs. It's valuable because the restriction that mixins can't have their own fields with initializers can make them harder to use without knowing this technique.
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.
Yes @munificent's example with an abstract getter is what I had in mind. Thanks for the excellent context! I agree with the plan to rewrite the examples, removing emphasis on on
.
I believe my misunderstanding about which is the super/sub-class came from the current wording you can restrict a mixin's use by using the on keyword to specify the required superclass
. @munificent's comment clarifies the strange behavior of on
. I agree it shouldn't be highlighted in the language tour, but maybe there should be some description somewhere since it's a keyword? Maybe a link to these comment threads? Maybe the on
keyword could be marked as deprecated or something to limit incorrect usage?
Maybe on
could be omitted from this mixin documentation but there could be on (exception)
and on (mixin)
entries in https://dart.dev/language/keywords, similar to final (variable)
and final (class)
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.
Maybe the
on
keyword could be marked as deprecated or something to limit incorrect usage?
It's not deprecated because it does have intended uses. They're just rare. You need an on
clause when you want to have a super
call inside the mixin, like:
class SomeBaseClass {
baseClassMethod() {
print('Base class!');
}
}
mixin SomeMixin on SomeBaseClass {
mixinMethod() {
print('Calling superclass method!');
super.baseClassMethod(); // <--
}
}
class SomeDerivedClass extends SomeBaseClass with SomeMixin {}
main() {
SomeDerivedclass().mixinMethod();
}
The on
clause exists to define the type that super
calls are resolved against. In the example here, the on SomeBaseClass
clause is what the compiler uses to know that super.baseClassMethod()
is OK.
@atsansone thanks for reaching out! Yes, this PR is more important to me than #5043, and I think it would be valuable to update the mixin docs. I'll write up what I'd propose for wording and example code. After playing with it a bit more, these are the 3 cases I'm thinking to cover:
|
@KyleFin : Apologies for this slipping between the cracks. Could you make your updates and I'll address this PR when they have been made. |
Closing in favor of #5694 , thanks everyone! |
I've long understood how mixins could provide value, but it took me way too long to understand how to access state in the super class (which makes mixins far more powerful). I'm not sure if this rewording is enough or if it may be helpful to add a simple example of a getter or accessing a member of the superclass in the mixin.
I like the brief docs, but this is a case where one more example would've gone a long way for me.