-
Notifications
You must be signed in to change notification settings - Fork 213
Should we support user-defined destructuring behavior and custom extractor pattern logic? #2104
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
Comments
I'm not sure I understand this. If a class implements a |
For positional fields, yes. That's a good point. But the current proposal doesn't give you any way of defining custom logic for destructuring to named fields. And having the main class implement |
Are we sure it should? I see the patterns proposal as a shorthand for code that would otherwise be written by the API user, not the author. Using a record pattern to extract named fields from a class is akin to writing out the fields manually. if (myVar is User) {
int age = myVar.age;
String username = myVar.username;
print("$age, $username");
} For the API author to be able to change that, such that writing a pattern is no longer equivalent to the above, is sure to be a source of confusion. At least, it's one more disconnect between appearance and behavior, and one more thing that can cause a breaking change. It's natural to need some explicit instruction on how to destructure positional fields, but named fields already have their names to go by. |
I agree. Maybe, instead of implementing So here is how I understand the current proposal to be: class CoordinatePair implements Destructure2<double, double> {
final double x, y;
const CoordinatePair(this.x, this.y);
@override
double field0 => x;
@override
double field1 => y;
}
class CoordinatePair3D extends CoordinatePair implements Destructure3<double, double, double> {
final double z;
const CoordinatePair3D(int x, int y, this.z) : super(x, y);
@override
double field2 => z;
} But by returning a // Some classes to mock the records feature
abstract class Destructure { }
class Destructure1<T> extends Destructure { }
class Destructure2<T1, T2> extends Destructure { }
class Destructure3<T1, T2, T3> extends Destructure2<T1, T2> { }
class Record2<T1, T2> implements Destructure2<T1, T2> { Record2(T1 x, T2 y); }
class Record3<T1, T2, T3> extends Record2<T1, T2> implements Destructure3<T1, T2, T3> { Record3(T1 x, T2 y, T3 z) : super(x, y); }
abstract class Destructurable<T extends Destructure> { T destructure(); }
// A 2D coordinate pair that can be destructured into (x, y).
class CoordinatePair implements Destructurable<Destructure2> {
final double x, y;
const CoordinatePair(this.x, this.y);
/// Returns a record with positional parameters
@override
Destructure2<double, double> destructure() => Record2(x, y);
}
// A 3D coordinate group that can be destructured into (x, y, z).
class CoordinatePair3D extends CoordinatePair {
final double z;
const CoordinatePair3D(double x, double y, this.z) : super(x, y);
@override
Destructure3<double, double, double> destructure() => Record3(x, y, z);
}
void main() {
final pair2d = CoordinatePair(1, 2);
final pair3d = CoordinatePair3D(1, 2, 3);
// The pair classes implement Destructure2 and Destructure3, so
// we can use pattern syntax to get the following:
var [x, y] = pair2d;
var [x, y, z] = pair3d;
} This also keeps the API clean as we don't have to add individual getters for each field, but rather one method to show Dart how it destructures. |
In general, I'm against introducing a new way to deconstruct objects. Just use getters! (And consider How and what to inspect in an object is something decided by the one doing the inspection, not the one writing the original class. The same thing for the Nothing prevents a class from having a In short: Putting the class itself in charge of destructuring is putting the responsibility in the wrong place. Allowing destructuring via getters allows the person doing the match to specify any public property of the class. (Might want to treat a class with |
We could make it even simpler than that since records with positional fields are already a proposed built-in type that implements a destructurable series of positional fields. So we could say that a class that wants to support positional destructuring simply returns a record with the desired positional fields. Like: // A 2D coordinate pair that can be destructured into (x, y).
class CoordinatePair implements Destructurable<Destructure2> {
final double x, y;
const CoordinatePair(this.x, this.y);
/// Returns a record with positional parameters
@override
(double, double) destructure() => (x, y);
}
// A 3D coordinate group that can be destructured into (x, y, z).
class CoordinatePair3D extends CoordinatePair {
final double z;
const CoordinatePair3D(double x, double y, this.z) : super(x, y);
@override
(double, double, double) destructure() => (x, y, z);
} The main downside is that it requires allocating a second object and copying all of the destructured positional fields over. Also, it's not consistent with named fields which are all accessed directly on the original matched object. |
I agree that calling getters on the object is the best default behavior.
But the original class does choose which getters to expose.
Yeah, I think that works pretty well for named field destructuring. But take a step back. Instead of thinking of destructuring as paralleling property access compare it to constructors. You can think of patterns as duals to expressions that produce values. Classes do control what constructor it provides and which parameters it expects. It make take parameters that do not precisely map to the public getters on the class. Also, the class may provide multiple different named constructors. All of that seems to be a useful way for a class to provide multiple expressive ways of building up an instance. Custom extractor methods and functions are the dual to that. An class Color {
final int red, green, blue;
Color(this.red, this.green, this.blue);
/// Allow accessing the color fields positionally since there is a natural
/// order.
static (int, int, int) extract() => (red, green, blue);
}
// Could also support free-floating extractor functions:
(int, int, int) hsv(Color c) =>
// Convert RGB to HSV and return tuple...
(int, int, int) lab(Color c) =>
// Convert RGB to Lab and return tuple... Then users could write code like: describeColor(Color c) {
case rgb(0, 0, 0): print('Black');
case rgb(255, 255, 255): print('White');
case hsv(_, 0, _): print('Monochrome');
case hsv(hue, _, _) if (hue > 80 && hue < 140): print('Greenish');
// Etc.
} |
That's what I meant. I was just trying to understand the subtyping relations between records so I made them into classes for testing in DartPad. Like how
I'm in agreement here, and I think @munificent's explicit naming of
It seems there are some cases where a "default" ordering is implied and supporting that can be a good thing. Like I am worried that in cases where multiple destructurings are valid, like |
I actually disagree that creating a tuple from an object is the dual of creating an object from a tuple (constructor and parameters), ... no matter how obvious it might seem. Going from a parameter list/tuple to an object is going from structural and transparent data with no given meaning and no behavior (no methods), to an abstraction which can hide data and exposes behavior and some properties. Going in the other direction removes, and potentially breaks, abstraction. That's bad, we don't want to encourage breaking abstraction. There is nothing inherently wrong with a class deciding to expose a tuple as a getter. Or expose an object combining multiple properties as a getter. The point is that there is no need to introduce new destructuring features to allow classes to expose things in ways that are meaningful to them, they can already use a getter. |
This is only for list patterns like: var [a, b, c] = someObject; Here,
Nit: Those would be: case (hsv: (_, 0, _): ...
case (hsv: (saturation: 0, ...)): ... The fact that you can already do this to call class-defined transformation methods is a good observation. I still think user-defined extractor functions/methods look nicer and feel more natural, but this does make me feel like it's maybe not as valuable as it would be in other languages where you don't already have a pattern syntax for calling arbitrary getters. |
I don't understand this. How does it break abstraction?
There is no need for multi-argument constructors either (or in fact single argument constructors, you can always just use a setter to initialize the fields after the fact). In the limit, the lambda calculus is enough, but I don't find these reductive arguments very useful. You need to talk about not just what is possible, but what the actual developer affordances look and feel like. |
A destructor ca neither provide no information which isn't already available through getters (in which case I think the destructor isn't really necessary because we have getter matching patterns), or it can provide information that isn't otherwise available, in which case I consider it breaking the abstraction. Destructuring is, in some way, a projection of the object state onto another state space. If the only thing that other state has going for it is that it's easy to write patterns against, then it feels like an implementation detail, and not something the class author would write by themselves. If the new state has a meaning by itself, like a HSV color, then it's a perfectly reasonable part of the abstraction, and should be there whether we had destructors or not. It's something we'd already have an I think we already have the affordances that destructors would give. I don't think they will make patterns much shorter or easier to write than just doing getter matching. They are a new concept, one which has no reason to exist other than as a an affordance for pattern matching, and I'm not convinced that affordance is much of a win, if any. |
This is where you're really losing me. I can, write now, add a method to a class which "destructs it" into some set of properties bundled as a class. For example, given a
An extractor is just the composition of the steps I described above. If you believe that one of those steps breaks the abstraction, you need to explain to me in what sense it does so, because I don't see how any of those steps break any abstraction.
Why?
I really don't understand this. Suppose I am a class author, and I am writing a collection. Aha! I say, I will make it easy for users to map a function over this collection. So I will write a map method! But no, you say: the only thing a map method has going for it is that it makes it easy to write maps over the collection. Users can already do that, that's what for loops are for! This is an implementation detail, and not something the class author should write themselves! In other words, when designing a class, authors often think about what affordances they want to provide to users of those classes. They may feel that providing specific iteration methods is useful. They may feel that providing transformation methods ( Or to put another way, whether you want class authors to think about this or not, they will. That is, if they must use the getter as you suggest above, then they still have to think about exactly the same concerns with exactly the same motivations, it's just a question of what syntax they use, right? Am I missing something? |
@munificent I don't understand this paragraph. There's nothing here that statically says that the return value will match the sub-pattern, right? So I don't think I understand how this can ever be a binder (irrefutable) pattern. Or am I missing/confusing something? |
@munificent
I would expect your example to look more like: describeColor(Description c) {
switch (c)
case Color(0, 0, 0): print('Black');
case Color(255, 255, 255): print('White');
case Color.hsv(_, 0, _): print('Monochrome'); // Not sure about this?
case Color.hsv(hue, _, _) if (hue > 80 && hue < 140): print('Greenish'); // Also not sure about this?
// Etc.
} The point is that part of the value here is that this is a way of programming the type extractor, so that in general you can do: doSomething(Expression e) {
switch (e) {
case UnaryOperator(var oper, var b) => ...
case BinaryOperator(var a, var oper, var b) => // do Stuff here
...
} and have the individual cases for the subclasses ( If I understand the suggestion from @lrhn , you would instead write this as: doSomething(Expression e) {
switch (e) {
case UnaryOperator(extract: (var oper, var b)) => ...
case BinaryOperator(extract: (var a, var oper, var b)) => // do Stuff here
...
} Is that right? One nice thing about this suggestion is that it gives you "named" destructors for free. That is, I can instead choose to structure my hierarchically with a single Operator subclass with different destructor getters and write this as: doSomething(Expression e) {
switch (e) {
case Operator(unary: (var oper, var b)) => // do Stuff here
case Operator(binary: (var a, var oper, var b)) => // do Stuff here
...
} which is not... terrible. |
The previous paragraph has:
Here's a concrete example of how this could be used: class Color {
final int r, g, b;
Color(this.r, this.g, this.b);
}
class HsvColor {
final int h, s, v;
static HsvColor extract(Color color) { // <-- Extractor.
// RGB to HSV conversion...
return HsvColor(h, s, v);
}
HsvColor(this.h, this.s, this.v);
}
void showHsv(Color color) {
var HsvColor(h: h, s: s, v: v) = color; // <-- Invoke as binder.
print('h:$h, s:$s, v:$v');
}
Yes, sorry, my previous Color example hypothesizes another kind of extractor (free-floating functions) and also just has a bug. Here's my initial code, with some remarks:
For the last two
Sorry for the confusion. Does that clarify? Basically, you can think of an extractor pattern as two steps:
|
@munificent I've always learned that when you have a static method that takes an instance of the surrounding class Would it be better to have, instead of declaring a static |
Or make class HsvColor { ... }
extension ExtractHsv on Color {
HsvColor asHsv() => ....;
// or, depending on perspective:
HsvColor toHsv() => ....;
} and pattern match as case {asHsv(): {h: 0, s: 0, v: 0}}: ... If the |
Currently with extension methods we can already cover most of this proposal, in what I think is an intuitive manner: describeColor(Description c) {
switch (c)
case Color(rgb: (0, 0, 0)): print('Black');
case Color(rgb: (255, 255, 255)): print('White');
case Color(hsv: (_, 0, _)): print('Monochrome'); // Not sure about this?
case Color(hsv: (hue, _, _)) if (hue > 80 && hue < 140): print('Greenish'); // Also not sure about this?
// Etc.
}
extension on Color {
(int, int, int) get rgb => (red, green, blue);
(int, int, int) get hsv {
final color = toHsv();
return (color.hue, color.saturation, color.value);
}
} I've used this already for translating some Scala code that uses a lot of unapply methods, and I think it is a great solution so far. The only thing this proposal gets you over that is to maybe have it look more like you are destructuring a different factory constructor interface, which I think is a good usability improvement as far as syntax overhead, but probably not worth the implementation effort relative to other features that are desired. (Personally, I'd rather see #2433 prioritized over this). However, this solution could use some documentation / official adoption of this pattern as the recommended solution to the user defined destructuring problem. |
The patterns proposal has "extractor patterns" that look like:
Foo(a, b, c)
In other words, like a function or constructor call, except the arguments are subpatterns instead of subexpressions. This syntax is part of pattern matching in most other languages that have patterns. In Dart the proposal says they work like this:
Destructure_n_
interface. For named fields, the pattern just directly invokes getters with the same name.This covers the common case of algebraic datatype style matches against a family of subtypes. And, for named fields in the record pattern, the class doesn't have to add any special support at all. It just works, for free.
I think this default built-in behavior is fine, but it's fairly limited. It might be nice if users could define custom behavior for how an object is matched or destructured. F# has active patterns and Scala has
unapply()
methods for this.An early version of the patterns actually had a design for this but for reasons I can't recall it didn't end up in the current version. I still think it's a good idea, so here's a simple proposed change:
Resolve the name on the extractor pattern to a type. If the name resolves to a type that declares a static method named
extract()
then:The method must take a single positional parameter. It is a compile-time
error if the static type of the value being matched is not a subtype of the
parameter's type.
When the pattern is evaluated, the method is invoked with the value being
matched. The method returns
null
to indicate a failed match. Returning anyother value is a valid match, and the resulting value is then matched
against the extractor pattern's record. (Typically, the
extract()
methodwill return a record, but it doesn't have to since any object can be matched
in a record pattern.)
If the return type of the method is non-nullable, then the extractor will
always match, so this
extract()
method can be used as a binder pattern.If the return type is nullable, it can only be used as a matcher pattern.
Otherwise, fall back to the current "type test and record match" behavior.
A couple of questions and thoughts:
Instead of making it a compile time error if the value's static type isn't
a subtype of the
extract()
method's parameter type, should it implicitlyinsert a type test and automatically fail if the value isn't of the
parameter's type?
Should we allow free-floating extractor functions by allowing the extractor
pattern name to resolve to a function declaration? What about resolving to
an instance method on the matched value's type?
Thoughts?
The text was updated successfully, but these errors were encountered: