-
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
"Specify members" and "access state" for mixins #5694
Conversation
Visit the preview URL for this PR (updated for commit 9282ec6): |
I'm done for the day and I'll be on vacation all week next week (and then working in AAR the week after that), but I'll take a look at this when I'm back. |
@MaryaBelanger, this looks great to me! I'm supportive of proceeding here and closing #4767. Thanks for picking this up! |
@munificent Friendly ping :) |
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.
One example needs filling in but otherwise LGTM!
src/content/language/mixins.md
Outdated
dependencies are defined for the mixin. | ||
|
||
```dart | ||
? |
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.
Don't forget to fill this in. :)
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.
Oh! I'm sorry, I think I actually needed help with this example, not a review yet 😬
Would it look something like... this? Not sure what would go in the mixin
class Person {
final String _name;
Person(this._name);
String greet(String who) => 'Hello, $who. I am $_name.';
}
mixin Impersonator implements Person {
// what goes in here?
}
class Impostor with Impersonator {
String get _name => '';
String greet(String who) => 'Hi $who. Do you know who I am?';
}
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 something like:
abstract interface class Tuner {
void tuneInstrument();
}
mixin Guitarist implements Tuner {
void playSong() {
// Should be in tune first!
tuneInstrument();
print('Strums guitar majestically.');
}
}
class PunkRocker implements Tuner with Guitarist {
void tuneInstrument() {
print("Don't bother, being out of tune is punk rock.");
}
}
?
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 don't think the class using the mixin also needs to implement Tuner, right? I wrote it like class PunkRocker with Guitarist {}
and that seems to work fine.
src/content/language/mixins.md
Outdated
dependencies are defined for the mixin. | ||
|
||
```dart | ||
? |
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 something like:
abstract interface class Tuner {
void tuneInstrument();
}
mixin Guitarist implements Tuner {
void playSong() {
// Should be in tune first!
tuneInstrument();
print('Strums guitar majestically.');
}
}
class PunkRocker implements Tuner with Guitarist {
void tuneInstrument() {
print("Don't bother, being out of tune is punk rock.");
}
}
?
This is an attempt to pick up #4767 and incorporate the this info-packed comment from #4834
It's pretty rough, I wasn't sure if "Specify members a mixin can call on itself" was an appropriate title for the new content, or if "accessing state" is interchangeable topic, or subtopic....?
I'm also totally missing an example (or anything more than a the single sentence about it, pulled from the above-linked comment).
@munificent hopefully this isn't totally off base? Lmk!
Apologies for overriding your work @KyleFin, I was having trouble working on your PR directly with the larger changes I wanted to do. Hopefully you can provide some feedback here!
I think this also fixes #4871