>> I agree, but it doesn't offer a strict guarantee. > > Do we need a strict guarantee more than we need unit tests?
I think we need both. Or rather, we need unit tests more, but if that doesn't exclude using finals, we can have both. >> Why not just use final, then? > > Because it makes it impossible to write unit tests. > > Maybe not impossible with the tool you pointed to. If that thing works, it > becomes: it requires effort to set up the CI infrastructure to allow this to > work, and we don't know who is going to do that, or when. Ah yes, I'm actually writing with the idea in mind that we'll be able to set up such a tool. If finals and mocking were mutually exclusive, I'd also prefer to have better tests. >> But making methods mockable while also keeping the >> capability to use finals is even better IMHO. > > If that can be made to work, sure. I'm just saying that an inability to mock far > outweights the potential benefits of declaring things as final. Yes, agreed with that. >> In theory, for sure. But I believe there are lots of occurrences in our >> code where static methods are not pure functions. > > Which indeed is one of the things we are currently trying to fix, because static > code can't be mocked. Indeed, and that's not so bad because it (hopefully) forces people not to write non-pure static methods. > Two sides of a coin, I think. Each of them has its benefits and its > drawbacks, I'd say. > > What benefits does it have to bind to a specific implementation that is not > guaranteed to stay as it is? If used properly, the final keyword should be for immutable implementations. Otherwise, it could be a questionable use case. > If somebody is volunteering to do the necessary work in the CI infrastructure, fine. > > To me it just seems like just removing the final modifier is the easier and > cheaper solution, and doesn't have any big downside. It depends on how hard is it to set up the library. Ideally, we only have to find a place where to enable it, nothing more. And I also think it won't harm existing tests in any way. It doesn't even require a CI change. The only possible drawback is performance / opcache integration pointed out by @simetrical. Let me try to upload a test change and see how it goes, tonight or tomorrow. Il giorno mer 28 ago 2019 alle ore 18:54 Daniel Kinzler < [email protected]> ha scritto: > Am 28.08.19 um 16:48 schrieb Daimona: > >> Subclassing should be very limited anyway, and even more limited across > > module > >> boundaries > > > > I agree, but it doesn't offer a strict guarantee. > > Do we need a strict guarantee more than we need unit tests? > > >> which could even be enforced via static analysis. > > > > Why not just use final, then? > > Because it makes it impossible to write unit tests. > > Maybe not impossible with the tool you pointed to. If that thing works, it > becomes: it requires effort to set up the CI infrastructure to allow this > to > work, and we don't know who is going to do that, or when. > > >> 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. > > That's what we are trying to fix, and final stuff is making it hard. > > >> 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. > > If that can be made to work, sure. I'm just saying that an inability to > mock far > outweights the potential benefits of declaring things as final. > > > In theory, for sure. But I believe there are lots of occurrences in our > > code where static methods are not pure functions. > > Which indeed is one of the things we are currently trying to fix, because > static > code can't be mocked. > > >> 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. > > What benefits does it have to bind to a specific implementation that is not > guaranteed to stay as it is? > > >> 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. > > If somebody is volunteering to do the necessary work in the CI > infrastructure, fine. > > To me it just seems like just removing the final modifier is the easier and > cheaper solution, and doesn't have any big downside. > > -- > Daniel Kinzler > Principal Software Engineer, Core Platform > Wikimedia Foundation > > _______________________________________________ > Wikitech-l mailing list > [email protected] > https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- https://meta.wikimedia.org/wiki/User:Daimona_Eaytoy "Daimona" is not my real name -- he/him _______________________________________________ Wikitech-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikitech-l
