Subclassing should be very limited anyway, and even more limited across
module
boundaries
I agree, but it doesn't offer a strict guarantee.
which could even be enforced via static analysis.
Why not just use final, then?
Method contracts should be enforced by compliance tests. When following
these principles, making
methods and classes final has little benefit.
Ideally, yes. But I don't think our codebase has enough tests for that.
Preventing mocking is however a pretty massive cost.
Definitely yes. But making methods mockable while also keeping the capability to use finals is even better IMHO.
Static code should only be used for pure functions. That's a very limited
use case.
In theory, for sure. But I believe there are lots of occurrences in our code where static methods are not pure functions.
If you want to make sure that any subclass won't ever change the implementation of a method, and thus all callers know what to expect from calling a final method. I see finals as a sort of safeguard to help write better code, like e.g. typehints.
This should eb done by documenting contracts, and having tests ensure
compliance
with these contracts. Final methods make callers rely on a specific implementation, which may still end up changing anyway.
Two sides of a coin, I think. Each of them has its benefits and its drawbacks, I'd say.
IMHO this would be a perfect compromise. I've filed T231419 for that,
and I
also think that before discussing any further, we should try to see if we can install that tool.
If I understand correctly, this would break as soon as the mock object
hits a
type hint of instanceof check. That won't fly.
No, that's only what happens with mockery. The tool I found just strips 'final' keywords from the PHP code - I believe, I still haven't looked at the implementation.
Il giorno mer 28 ago 2019 alle ore 16:29 Daniel Kinzler < dkinzler@wikimedia.org> ha scritto:
I see no good use for final methods or classes. Or rather: I see a very limited benefit and a pretty massive cost.
Subclassing should be very limited anyway, and even more limited across module boundaries, which could even be enforced via static analysis. Method contracts should be enforced by compliance tests. When following these principles, making methods and classes final has little benefit. Preventing mocking is however a pretty massive cost.
I'd just remove the final markers. But maybe I'm overlooking something?...
Am 28.08.19 um 10:38 schrieb Daimona:
I don't like these limitations either, but testing is an integral part of development, and we need to code in a way that facilitates testing.
This is especially true for e.g. static methods, but here we'd be renouncing to a possibly useful feature.
Static code should only be used for pure functions. That's a very limited use case.
Why do methods ever "have" to be final?
If you want to make sure that any subclass won't ever change the implementation of a method, and thus all callers know what to expect from calling a final method. I see finals as a sort of safeguard to help write better code, like e.g. typehints.
This should eb done by documenting contracts, and having tests ensure compliance with these contracts. Final methods make callers rely on a specific implementation, which may still end up changing anyway.
That would be a nice solution if it works well. If someone wants to volunteer to try to get it working, then we won't need to have this discussion. But until someone does, the question remains.
IMHO this would be a perfect compromise. I've filed T231419 for that,
and I
also think that before discussing any further, we should try to see if we can install that tool.
If I understand correctly, this would break as soon as the mock object hits a type hint of instanceof check. That won't fly.
-- Daniel Kinzler Principal Software Engineer, Core Platform Wikimedia Foundation
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l