dart-lang / language

Design of the Dart language
Other
2.66k stars 205 forks source link

Allow this.{field} in the constructor parameters to refer to fields declared in a mixin. #3242

Open thijsdaniels opened 1 year ago

thijsdaniels commented 1 year ago

I have no experience with writing compilers or anything, so what I'm suggesting might not be possible or a good idea. But what I would like is for this to be supported:

class Document {
  String id;
}

mixin Owner on Document {
  User user;
}

class Consumption extends Document with Owner {
  Consumption({
    required super.id,
    required this.user, // This is currently not allowed, because `user` is not declared in `Consumption`.
  });
}

Below is my reasoning as well as ChatGPT's 2 cents 😂

Conversation with ChatGPT # User if this is allowed in dart ```dart class Document { String id; } mixin Owner on Document { late User user; } class Consumption extends Document with Owner { Consumption({ required super.id, required User user, }) { this.user = user; } } ``` then apparently it considers `this.user` to be a member of the `Consumption` class. so then why is this not allowed? ```dart class Document { String id; } mixin Owner on Document { User user; } class Consumption extends Document with Owner { Consumption({ required super.id, required this.user, }); } ``` # ChatGPT The syntax `this.something` in a constructor refers to a field declared in the current class itself, not in the superclass or in the mixin. The reason for this is that each class in Dart owns its own fields. In the first piece of code you've shared, you are using `this.user = user;` which is allowed because you're assigning a value to the user field from within the body of the constructor. In this case, `this.user` refers to the user field from the Owner mixin that Consumption is using. ```dart Consumption({ required super.id, required User user, }) { this.user = user; // This refers to the `user` field from the `Owner` mixin } ``` In the second piece of code, you are trying to use `this.user` in the parameter list of the constructor, which is not allowed because `this.user` refers to a user field declared within `Consumption`, not within the `Owner` mixin. There is no `user` field within `Consumption`, hence the error. ```dart Consumption({ required super.id, required this.user, // This would refer to a `user` field within `Consumption` }); ``` While a mixin's fields can be accessed and manipulated from within a class that uses the mixin, the fields do not actually belong to the class itself, they belong to the mixin. Therefore, they cannot be used with this in the parameter list of a constructor. To conclude, when you use `this.something` in a constructor's parameter list, Dart is expecting something to be a field declared directly within the current class, not in a superclass or mixin. # User but they _can_ be used with `this` in the _body_ of the constructor. that's confusing right? why not just make it consistent and allow using `this.user` in the constructor parameters as well? wouldn't that be a good feature for dart to build? # ChatGPT It's important to clarify that this.something is always a reference to a member variable of the current class. However, when you're inside a method body (or constructor body), you have access to any variable that's in the current scope, which includes member variables of the current class, superclasses, and any applied mixins. In the initializer list of a constructor (where you have required this.user), the Dart language has a more limited scope. Here, this.something specifically refers to an instance variable of the current class and does not consider any superclasses or mixins. This distinction isn't unique to Dart, and many object-oriented languages behave in a similar way when dealing with constructor parameters versus method (or constructor) bodies. The feature you're proposing - allowing this in the constructor parameters to include superclasses and mixins - could potentially lead to issues. For example, what if the superclass, the subclass, and a mixin all have a field with the same name? Which one would this refer to? This could lead to ambiguity and potential bugs in code. So, while it might seem like it could make things simpler in some cases, such a feature would also introduce a range of potential issues that could complicate things and lead to confusion or bugs. That said, the Dart language is constantly evolving, and its designers are always considering ways to make the language more powerful, flexible, and easy to use. # User but doesn't this ambiguity also hold up in the body of the constructor? If I have 2 mixins that both declare a `user` member, the body of the constructor would have the same problem understanding what `this.user` refers to right? # ChatGPT You're correct that if two mixins both declare a user field, there would be a naming conflict which could lead to ambiguity. However, the Dart language prevents this situation outright. Dart does not allow you to apply two mixins to a single class if both of them declare a member with the same name. If you try to do this, the Dart analyzer will give you an error, and your code won't compile. This is one of the ways Dart maintains consistency and prevents potential bugs related to naming conflicts. If you want to use two mixins that have fields or methods with the same name, you'll have to resolve this conflict manually, for example by renaming the fields or methods in one of the mixins. So in the body of the constructor, this.user is unambiguous because Dart enforces that there can only be one user field in the scope of the class, whether it's declared in the class itself, in its superclass, or in any of its mixins. # User well then if dart enforces rules that prevent this ambiguity, then doesn't that mean that the ambiguity also wouldn't arise in the initializer list of the constructor? # ChatGPT Technically speaking, if Dart extended the scope of this in the constructor initializer list to include fields from superclasses and mixins, and continued to enforce the rule that no two fields can have the same name, then yes, there wouldn't be any ambiguity about what this.something refers to. However, the reason this isn't the case has more to do with established conventions and principles in object-oriented programming. In Dart, as well as many other languages, this in the context of a constructor refers specifically to the current class being constructed. It's used to initialize fields that belong to that class. Extending the scope of this to include superclasses and mixins could make code harder to understand and maintain. It might not be immediately clear, especially in a large codebase, whether a field is part of a class, part of its superclass, or part of a mixin. So while the language could technically support this feature without causing ambiguity, there are good reasons for the current design. As always, the design of programming languages involves trade-offs and there is often no one "right" solution. It's about finding the balance that best meets the needs of the language's intended use cases and the principles the language designers have set out to follow. # User if it's _just_ there for convention, then that convention might be worth breaking. dart has already introduced a lot of features that are unconventional in other languages. # ChatGPT You raise a valid point. The conventions and paradigms that guide the design of a language aren't set in stone and can change over time to accommodate new ideas, solve emerging problems, or simplify certain aspects of coding. In fact, Dart itself has undergone significant changes since its inception. For example, Dart 2.0 introduced strong typing and null safety, which were departures from the conventions of Dart 1.0 but were implemented to address specific issues and make the language safer and easier to use. As such, the idea of extending the scope of this in the initializer list of constructors to include fields from superclasses and mixins could be a valuable proposal for future versions of Dart. If you believe this feature would significantly improve the language, you could consider submitting a proposal to the Dart language team. At the end of the day, the evolution of a programming language is a community-driven process. The feedback and ideas of developers play a crucial role in shaping the future directions of the language. Just keep in mind that any changes would have to be carefully considered in terms of their impact on the language as a whole, including potential edge cases, compatibility issues, and effects on the readability and maintainability of code.
eernstg commented 1 year ago

What you'd do if you want a mixin to have state is (1) make it abstract, or (2) initialize it lazily, or (3) initialize it in the declaration.

Make it abstract:

class User {}

class Document {
  String id;
  Document({required this.id});
}

mixin Owner on Document {
  abstract User user;
}

class Consumption extends Document with Owner {
  @override
  User user;

  Consumption({
    required super.id,
    required this.user,
  });
}

You will be reminded about the need to declare the actual state corresponding to any abstract instance variables your superinterfaces may declare, so there's no danger that you will forget to make it concrete.

It is somewhat inconvenient that you have to, essentially, duplicate the declaration of User, but otherwise this is a very clean solution.

You can also initialize it lazily:

class User {}

class Document {
  String id;
  Document({required this.id});
}

mixin Owner on Document {
  late User user;
}

class Consumption extends Document with Owner {
  Consumption({
    required super.id,
    required User user,
  }) {
    this.user = user;
  }
}

This is a bit more dangerous, because there will be no heads-up from static analysis if you forget to initialize user in a constructor (which means: in some declaration which isn't in the mixin, that is: far away from the declaration of user).

You will get a dynamic check: If user is ever used, and it wasn't ever initialized, a dynamic error occurs. But "when did we not do this particular thing?" is always hard to debug. ;-)

Finally, you could initialize it in the declaration itself:

class User {}

class Document {
  String id;
  Document({required this.id});
}

mixin Owner on Document {
  User? user; // Implicitly initialized to null.
}

class Consumption extends Document with Owner {
  Consumption({
    required super.id,
    required User user,
  }) {
    this.user = user; // We probably still want something like this.
  }
}

Again, you won't get a heads-up if you forget to initialize it, and it can be difficult to make the distinction between "this user has not been initialized" and "this user has the value null because we did user = null, and it is correct for user to have that value". Also, when you use a nullable variable type it gets less convenient to use, because you'd have user?.something() all over the place, and that might introduce null values (according to the static analysis, even if not in practice) all over the place.

(I won't comment on ChatGPT's responses. Don't bet your life on them, though. ;-)

lrhn commented 1 year ago

The long term solution would be to allow mixins to have constructors, but that's a larger language change.

thijsdaniels commented 1 year ago

@eernstg Thanks for the detailed examples! I was aware of the late and nullable approaches, but hadn't considered making it abstract yet. I think i'll do that for now.

However, none of the three solutions are ideal in my opinion, exactly for the reasons @eernstg described. Having to re-declare the field in every implementation is cumbersome (i feel really lazy saying that ;P but it can become a hassle to refactor) and the late and nullable approaches make it possible to forget initializing and/or will require null checks throughout the project.

So I think making it abstract is the best solution at the moment, but I have a feeling that it could be better.

jakemac53 commented 1 year ago

Note that if you make it abstract, you very likely only want to declare an abstract getter:

abstract User get user;

That allows implementations to make it a final variable etc, and is probably what you actually want.