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

Mixins article is scary and out of date, remove? #946

Closed
chalin opened this issue Jun 20, 2018 · 8 comments
Closed

Mixins article is scary and out of date, remove? #946

chalin opened this issue Jun 20, 2018 · 8 comments

Comments

@chalin
Copy link
Contributor

chalin commented Jun 20, 2018

From @sethladd on June 20, 2018 8:32

A user just approached me, asking about Mixins. We saw https://www.dartlang.org/articles/language/mixins but there's a scary red banner. And it's written for language spec people and not users. And it's probably not updated. Should we just remove this article? We can redirect over to the Mixins section of Language Tour.

Copied from original issue: dart-lang/site-webdev#1657

@chalin
Copy link
Contributor Author

chalin commented Jun 20, 2018

That page has that banner because it has yet to be updated as a part of #407.

I believe that the article content is still valid as is, given that the only Dart 2 related change mentioned in the spec relative to mixins is the following:

Update mixin application forwarding constructors to correctly handle optional parameters and const constructors.

@eernstg, WDYT?

I'd vote to keep the article. I can do the work necessary to ensure that the code excerpts are tested, and then remove the banner.

Thoughts? @sethladd @kwalrath

@sethladd
Copy link

Things I'd update:

  • remove mention of Atom
  • remove mentions of "previous implementations" or "Dart 1.12" ... that's very very old by now, and is confusing for any new developer
  • Define the audience for this doc. Is it for language geeks? Common developers? Phrases like "
    If you are familiar with the academic literature on mixins you can probably skip this section. Otherwise, please do read it, as it defines important concepts and notation. Those wishing to delve deeply into the topic can start with this paper: Mixins in Strongtalk." aren't very welcoming. If this doc is purely about the theory of mixins, call it "Theory of Dart Mixins" and add a header: "If you just want to know what mixins are and how to use them, go to the language tour... this doc is about the theory and history"
  • Consider non-DOM example code, appealing to a more generic audience

@chalin
Copy link
Contributor Author

chalin commented Jun 20, 2018

Thanks Seth, that is valuable feedback!

Any others have an opinion about revising vs. dropping this article?

@eernstg
Copy link
Member

eernstg commented Jun 20, 2018

First, --supermixins should not be announced widely today because that feature is going away. We will soon introduce a separate mixin declaration which will enable similar features in a more well-defined setting.

I do think that the contents of the article is very nearly valid today, but also that there are many little things that have changed. For example:

Mixins are implicitly defined via ordinary class declarations.
In principle, every class defines a mixin that can be extracted from it.

With the new mixin declaration as the (strongly!) preferred approach, mixins are defined separately. We will probably preserve the ability to use a class with superclass Object to obtain a mixin (for backward compatibility) for some time, but we should describe mixins as primarily a separate construct.

However, in this proposal, a mixin may only be extracted from a class
that has no declared constructors.

Which proposal is that?

We have plans for supporting constructors in mixins, but this will not be part of the first version of the mixin declaration feature. However, it will simply be a syntax error (in that first version) to declare a constructor in a mixin, so there's no need for developers to think about it in terms of "can I extract a mixin from this entity or not?", it will be a mixin, or we shouldn't even consider using it as a mixin.

Summing up, it seems reasonable to mark the mixins article as "not up to date" in some way (and references to it ought to be given in a context where that wouldn't be a complete surprise), but it also seems reasonable to refer to it for conceptual background information.

This makes me think that the safe and easy way out is to keep the article marked and change the "novice oriented" references to point to the language tour (and then we must make sure that the language tour says something which is true, but that would presumably be a shorter text).

@eernstg
Copy link
Member

eernstg commented Jun 20, 2018

PS: I was also wondering about this:

abstract class Collection<E> {
  Collection<E> map((f) {
    var result = newInstance();
    forEach((E e) { result.add(f(e)); });
    return result;
  });
  ...
}

It looks like a combination of an abstract method declaration and an invocation thereof.

@sethladd
Copy link

First, --supermixins should not be announced widely today because that feature is going away. We will soon introduce a separate mixin declaration which will enable similar features in a more well-defined setting.

If that's true, then we should delete this article. The Language Tour will be sufficient for those wishing to use mixins in the meantime.

Less to maintain, less to update, less confusing.

@eernstg
Copy link
Member

eernstg commented Jun 29, 2018

Can't we keep it and mark it as historic? It's background information about mixins by the inventor of mixins..

@chalin
Copy link
Contributor Author

chalin commented Jun 29, 2018

I'm with Erik on this one. I believe that we can address the original concerns while keeping the article, w/o too much effort. I'll put a PR together for all to comment on.

chalin added a commit that referenced this issue Jun 29, 2018
chalin added a commit that referenced this issue Jul 3, 2018
chalin added a commit that referenced this issue Jul 3, 2018
* Clarify scope and readership of "Mixins in Dart"

Fixes #946

* Change title, fix lang. tour reference

* Updates based on Kathy's feedback

* Prose tweak

* Adjustments

* Final tweak
@atsansone atsansone removed the docs label Oct 18, 2023
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

No branches or pull requests

4 participants