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

"Specify members" and "access state" for mixins #5694

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

MaryaBelanger
Copy link
Contributor

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!

@dart-github-bot
Copy link
Collaborator

dart-github-bot commented Apr 5, 2024

Visit the preview URL for this PR (updated for commit 83a6a97):

https://dart-dev--pr5694-clarify-mixins-ltjhm8cs.web.app

@munificent
Copy link
Member

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 MaryaBelanger added review.tech Awaiting Technical Review review.await-update Awaiting Updates after Edits labels Apr 8, 2024
@KyleFin
Copy link
Contributor

KyleFin commented Apr 12, 2024

@MaryaBelanger, this looks great to me! I'm supportive of proceeding here and closing #4767. Thanks for picking this up!

@MaryaBelanger
Copy link
Contributor Author

@munificent Friendly ping :)

Copy link
Member

@munificent munificent left a 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!

dependencies are defined for the mixin.

```dart
?
Copy link
Member

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. :)

Copy link
Contributor Author

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?';
}

Copy link
Member

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.");
  }
}

?

src/content/language/mixins.md Show resolved Hide resolved
### Use the `on` clause to declare a superclass

The `on` clause exists to define the type that `super` calls are resolved against.
So, you should only use it if you need to have a `super` call inside a mixin.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good summary!

it to define the abstract method upon which its behavior depends.

```dart
abstract mixin Musician {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No abstract needed (or allowed) here.

dependencies are defined for the mixin.

```dart
?
Copy link
Member

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.");
  }
}

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review.await-update Awaiting Updates after Edits review.tech Awaiting Technical Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants