2

By default, in Apex, unlike Java and other OOP languages, all classes and methods are final.

You aren't allowed to make this explicit and if you want things not to be final, you need to use the keyword "virtual".

This seems an abnegation of S.O.L.I.D., or at least of the Open/closed principle.

As a developer who values the test pyramid, and therefore creates true unit tests (in addition to integration tests), this requires me to create wrappers for many native SFDC classes as well as to litter my code with instances of the keyword "virtual" in order that I can mock out, spy upon, and otherwise control any code which I'm not targeting with my tests.

One might say this is making the code noisy and, to look at typical Salesforce code, one might say my code is not "idiomatic", even though it is obviously relying on features supported by the language.

Are there any good reasons I should be ignoring S.O.L.I.D. principles and leaving my classes and methods as final?

closed as primarily opinion-based by battery.cord, Pranay Jaiswal, gNerb, Sebastian Kessel, glls Jun 18 '18 at 18:05

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

11

Once you expose a class as virtual or abstract, that means that you can extend it. There's two basic camps out there, those that think that things should be final by default, and those that don't. One logical argument I found has this to say:

Well, a non-final class can be extended of course! Any public or protected methods can be overridden and protected fields read/written. More importantly, you cannot reverse the decision — i.e. once a public non-final class, always a public non-final class. In contrast, when using final as the default for classes, you can reverse your decision — i.e. you can always open, but you can never close.

Do some research, you'll find more arguments like this. If a class is final by default, you can assure yourself it's not being used anywhere else--and most classes never will be. In contrast, once you open it up for subclassing, you have introduced a new layer of complexity for that class for all time (it's now open). The page above also says:

Josh Bloch in his (also excellent) book Effective Java states “you should make each class or member as inaccessible as possible”.

Apex Code makes this easy for developers. I've written thousands of Apex Code classes[citation needed], and I can likely count the number of times that I've needed non-final classes on both hands. They're actually really super-rare to see in practice, so it should be the default model (and thus, it is!).

You can think of virtual/abstract as "more permissive" than final, because you can poke in to it. However, as far as testing goes, we have the Stub API for mocking objects, and we have @TestVisible Annotation for reaching into an object only during testing. Between these two features, it's almost never necessary (if ever) to have your classes virtual by default just for test dependency injection. It makes all your code easier to read and maintain.

  • 1
    I've looked at the Stub API and I find it to be a very ugly solution. To me, it makes much more sense to either use a common interface or extend an existing class. Of course, this would requires us to add either "virtual" or interface references which are only needed for tests, which is also an ugly solution, but more intuitive and elegant. – Brian Kessler Jun 18 '18 at 23:31
  • Great response regarding the debate over "final", by the way... I've been digging into the argument a bit and your quotes above are quite compelling. Particularly since I prefer composition over inheritance and (even while I've been doing it) never really liked the idea of making code virtual just to make it more testable (I'm still not happy with the Stub API though it helps that I now better understand why it is that way). – Brian Kessler Jun 19 '18 at 7:05
9

I follow the YAGNI principle so do not add the clutter up front. If I later want to sub-class then the clutter is added then.

Further, making code more complex to make it testable is something I would rarely do. Can't find it now, but I remember a great article where an elegant web framework was made entirely inelegant to make the code more unit testable. (By elegant I mean easy to understand and modify in the future which is a core characteristic of good code.)

Yes it would be great if Apex didn't have some of the design features that make coding painful but it does. So rather than trying to apply techniques that work well with other languages, I think it is best to accept the limitations and make pragmatic decisions.

One further thought about tests is that the tests should focus on the public behaviour of classes not their implementation detail. (This again ties in with ease of modification: the tests define the required behaviour not the accidental behaviour of the implementation.) So even @TestVisible should only be used where absolutely necessary.

  • 1
    Making pragmatic decisions and asserting public behavior are two very strong points in working in this ecosystem. As enterprise apps move into more SPA like behavior, serverside state is becoming irrelevant. Testing the inputs and outcomes are much more important. On the side of pragmatic decisions, we need to write code that is easily human readable, easily human manageable since this ecosystem swaps developers / admins / resources at the drop of a hat. You must keep the business costs, market resource skillsets, in mind at all times. – tsalb Jun 18 '18 at 17:45

Not the answer you're looking for? Browse other questions tagged or ask your own question.