overridden_fields documentation is vague

[jamesderlin]

https://dart-lang.github.io/linter/lints/overridden_fields.html states:

Overriding fields is almost always done unintentionally.

If @override is used, it quite clearly is not unintentional.

Regardless, it is a bad practice to do so.

Why is it a bad practice to do so? It would be nice to provide examples. https://github.com/dart-lang/linter/issues/1109 provides some arguments. However, I don't fully buy some of them.

• From the perspective of an API consumer, including the derived class, fields are indistinguishable from a getter and setter.
• Getters and setters are functions.
• Overriding functions is okay.

Are we also saying that overriding get and set functions from a base class also is bad? What about overriding a base class field with get and set functions?

[pq]

/cc @lrhn @eernstg @leafpetersen @munificent

[lrhn]

Overriding a field with another field is rarely the thing you want to do because it means the object has two storage locations for what is effectively one property. I'm sure I can come up with a situation where it makes a kind of sense, but it's going to be highly speculative. If you further make the super-class field completely unreachable by not providing any super.name access to the original, then the original is definitely unusable, which is unlikely to be a good deliberate design. You'd likely be better off implementing the interface than extending the superclass.

Overriding a field with a getter or setter which calls the super-getter/setter and then does something more, that's completely reasonable.

So is overriding a getter/setter pair with another getter/setter, and unless the subclass is in the same library as the superclass, it shouldn't even know whether the superclass uses a field with implicit getter/setter or an explicit getter/setter declaration.

Whether something is a fields is an implementation details, it should be able to change from version to version of a package without anyone noticing. Blindly warning about overriding a field, but not a getter, breaks this abstraction.

(If it's possible to detect that a field is overridden and unreachable, and it's declared in the same library, then there might be a good reason to give a warning, because you're in position to actually do something about it. Anything other than that is risking false positives.)

[moneer-muntazah]

Thank you both for the post. @lrhn I came across this discussion after coming up with this idea, and getting the lint rule.

class Product {
  Product({required this.sku});
  final String sku;
}

class CustomProduct extends Product {
  CustomProduct({required Product product, required String customSku}) : sku = 
    customSku, super(sku: product.sku);

  @override
  final String sku;

  Product get parent => Product(sku: super.sku);
}

The real example is more complicated and has more fields, and methods, but the idea is that my CustomProduct will sometimes need to give back the original Product, but needs to be treated by the pluming of my app as a Product. Would you recommend overriding fields in this instance? And if not why. I realize that the idea looks a bit convoluted which is why I'm hesitant to implement it, but I don't want to change the pluming so it's very convenient for me to subclass.

Thank yo again

[lrhn]

I'd probably prefer composition over inheritance, so:

class CustomProduct extends Product {
  final Product parent;
  CustomProduct({required Product product, required String customSku})
      : parent = product,
        super(sku: customSku);
}

This way there is no overriding the sku field, and everything except the parent getter is a completely plain Product.

Alternatively, if you really want to store the parent SKU instead of the parent object, I'd do:

class CustomProduct extends Product {
  final String _parentSku;
  CustomProduct({required Product product, required String customSku})
      : _parentSku = product.sku,
        super(sku: customSku);
  Product get parent => Product(sku: _parentSku);
}

Either is easier to read than the unnecessarily overriden sku field.

You generally never need to override a field with another field. Overriding with a getter/setter with side effects makes sense. Overriding a final field with a mutable field makes a kind of sense, but feels icky, I'd probably prefer reimplementing the interface instead. Only ever do it for classes where you also wrote the superclass, because otherwise you can't be sure the superclass doesn't do super.field in some methods and this.field in other methods to get to the field. Your override won't affect super.field. In those cases, I'd make a skeleton base class instead, where the field getter is abstract, and provide different subclasses of that instead.

[filiph]

Here's something I tried to do at least a couple of times:

class A {
  String field = "A generic default initial value for field.";
}

class B extends A {
  @override
  String field = "An initial value that makes more sense for the subclass.";
}

For what it's worth, I didn't care about A.field becoming unreachable. I didn't even realize it's still there! I just thought I'm overriding the initialization.

To avoid the lint, which I never fully understood until now, I learned to do the following:

class A {
  late String field = fieldDefault;

  String get fieldDefault => "A generic default initial value for field.";
}

class B extends A {
  @override
  String get fieldDefault => "An initial value that makes more sense for the subclass.";
}

(I realize I could do it in the constructor without introducing a new getter, but that can make the classes a bit harder to maintain, imho.)

Anyway, I just thought I'd add this point. Maybe I'm the only one who wrongly assumed that overriding a field like this is just giving it a different initial value in the subclass. If not, does it make sense to mention this misconception in the lint warning?

[bwilkerson]

Yes, it makes sense to mention it. Thanks for pointing this out.