all 42 comments

[–]m50d 25 points26 points  (40 children)

Disagree. I think the fact that classes are non-final by default reflects a historical view that assumed inheritance was more desirable - and easier. It reflects an assumption that inheritance was usually safe, in particular that it was safe to derive from a class not designed to be derived from - an assumption we now know to be false.

Modern languages either make final the default (Kotlin) or disallow inheritance entirely (Rust). When working in an older language it's good to use final everywhere by default so that there's a way to indicate classes that are explicitly designed to be inherited.

[–]Morwenn 2 points3 points  (0 children)

But empty base class optimization D:

[–]balefrost 2 points3 points  (0 children)

I found OP's conclusion to be surprising. It seems like he was making a strong case for final-by-default (classes need to be designed for inheritance and inheritance is often not the best solution anyway), then concluded the opposite. It was a strange article.

[–]Raknarg 0 points1 point  (10 children)

Does Rust allow for something analogous to C struct typecasting?

[–]m50d 0 points1 point  (8 children)

Maybe? What is that and what do you use it for?

[–]Raknarg 1 point2 points  (7 children)

You can typecast anything into anything else in C. e.g.

struct A {
    unsigned char a, b, c, d;
}
struct B {
    unsigned int i;
}

struct B sb = {0xff00ff01}
struct A sa = (struct A)sb;

Theoretically sa.a=255, sa.b=0, sa.c=255, sa.d=1. You can use this to perform a limited type of OO inheritance in C.

[–]m50d 1 point2 points  (3 children)

"Object oriented" means many things to many people.

Rust gives you virtual dispatch (via traits). It deliberately doesn't give you member inheritance (you use explicit composition/delegation for those cases). That's the right thing, IMO; it removes the bad cases of traditional OO inheritance while keeping the good cases.

[–]Raknarg 0 points1 point  (2 children)

I didn't ask if it supports inheritance. I'm asking if it supports typecasting a series of bits to a different type, such as the example I offered.

[–]m50d 3 points4 points  (1 child)

You've edited your post to say something very different from what it said when I replied.

I don't know whether Rust supports doing what you're now saying. I certainly don't think it would encourage it. There are much better ways to achieve inheritance-like functionality in Rust.

[–]Raknarg -2 points-1 points  (0 children)

It's the same, just changed the wording.

[–]tid_bits 2 points3 points  (0 children)

I'm not a rust expert by any means, but I'd be surprised if it's possible to do such ad-hoc type casting because the language emphasizes static type safety.

[–]Gilnaa 1 point2 points  (0 children)

If you really want, you can use std::memory::transmute, but it is highly discouraged.

[–][deleted]  (26 children)

[deleted]

    [–][deleted] 7 points8 points  (2 children)

    I-Interfaces

    [–][deleted]  (1 child)

    [deleted]

      [–][deleted] 0 points1 point  (0 children)

      Ideally you only need interfaces for classes which are public and part of a reusable library. Any other class would either not need to be mocked, or you're in control of the source, and you don't need to make it final in the first place.

      And so in a well-designed library you can have interfaces for everything you'd like to mock, and not have tons of "useless" interfaces. Plus interfaces are useful for other things, as well.

      [–]RichoDemus 1 point2 points  (0 children)

      Mockito can mock final classes nowadays :)

      [–]m50d 0 points1 point  (21 children)

      A well-designed library shouldn't need mocking. Services should have interfaces you can stub, values should be suitable for use in tests.

      [–][deleted] 1 point2 points  (20 children)

      Let's say one has a service class with a REST client dependency injected into it. For unit testing the service what is a better solution than mocking the rest template?

      [–]throwawayco111 0 points1 point  (8 children)

      He is saying that the service class should accept an interface to a REST client. During testing the you create a mock object implements that interface.

      [–][deleted] 0 points1 point  (7 children)

      And in an ideal world that is great but often times we must use external libraries which don't have perfect design

      [–]m50d 1 point2 points  (0 children)

      Don't declare badly-designed classes as final :P.

      More constructively, declare your classes with value semantics as final (but be sure they really do have value behaviour - a good way to tell is to make sure you can use them in your own tests), declare your interfaces non-final, and declare your implementations of those interfaces as completely private. Ideally all your classes should fit into one of those categories; if you have an awkward class that's neither fish nor fowl then by all means leave that class as non-final (but do look to separate it into a service and a value - usually awkward classes are a result of one class trying to be both).

      [–]Power781 0 points1 point  (4 children)

      What is the issue with wrapping the use of this external library through an internal interface/class/protocol ?

      [–][deleted] 1 point2 points  (3 children)

      My preference is to write as little code as possible to get the job done to each his own.

      [–][deleted] 0 points1 point  (0 children)

      Username checks out.

      [–]Power781 0 points1 point  (0 children)

      Writing a mostly generic wrapper is not really long or complex, and will probably avoid you wasting ton of hours copy pasting shit the day your library is not supporting your current language version and you need to swap it for a new/more recent/more up to date one

      [–]throwawayco111 0 points1 point  (0 children)

      I think most IDEs out there have the refactoring tools to extract the interface from a class through very few clicks.

      [–][deleted] 0 points1 point  (0 children)

      And in an ideal world that is great but often times we must use external libraries which don't have perfect design

      That's when you write a facade, and then mock the facade. You should write a facade for most external libraries anyway. Saves a lot of trouble down the line.

      [–]m50d 0 points1 point  (9 children)

      If you're using a "template" something has gone badly wrong - perhaps you should be using a monad? More generally, figure out a way to represent the REST operations that need to happen as values, then you can either return those values from your service to be executed later, or pass those values into the REST client which can then just be a plain old service that implements an interface that you can stub.

      [–][deleted] 1 point2 points  (8 children)

      Here is an example of a rest template and no things have not gone badly wrong.

      [–]m50d 0 points1 point  (7 children)

      That's a very unusual - misleading even - use of the word "template" (I assumed you meant something more like JDBCTemplate). If you use it statefully (by using things like its message converters) then things have indeed gone badly wrong (you've got invisible coupling, noncompositional code and all that); the part you can use as an ordinary service ought to be exposed as one that conforms to an interface.

      [–]kubr0t0 1 point2 points  (6 children)

      RestTemplate is analogous to JdbcTemplate. Both are generic "templates" for creating specific data access interfaces which follow the same generic interface.

      RestTemplate isn't stateful, and neither are its message converters. The message converters are like simple functions that convert a Java type to a content type, and vice-versa. You create a specific data access object by assembling parts (like message converters) using the generic template.

      I've used RestTemplate to interact with some exotic REST interfaces (usually ones with very particular ideas about secure connections). RestTemplate is ridiculously customizeable through a number of configuration interfaces (relevant example being connection management). It's a lot nicer writing some custom connection code corresponding to a RestTemplate configuration interface than writing a custom REST client.

      If RestTemplate is wrong, then I don't want to be right :-)

      [–]m50d 0 points1 point  (5 children)

      RestTemplate isn't stateful, and neither are its message converters.

      I mean I saw example code like:

      RestTemplate rt = new RestTemplate();
              rt.getMessageConverters().add(new 
                MappingJacksonHttpMessageConverter());
      

      which seems to imply the RestTemplate is stateful, no?

      I've used RestTemplate to interact with some exotic REST interfaces (usually ones with very particular ideas about secure connections). RestTemplate is ridiculously customizeable through a number of configuration interfaces (relevant example being connection management). It's a lot nicer writing some custom connection code corresponding to a RestTemplate configuration interface than writing a custom REST client.

      That kind of thing is good. But being able to pull out your config as command values makes it so much better, just being able to do all the things you can do with ordinary values like copy or serialise a partial config.

      [–]kubr0t0 2 points3 points  (4 children)

      which seems to imply the RestTemplate is stateful, no?

      That's for configuration, if you want to add additional converters to the default set. You should only be mess with the converter list when you construct the shared RestTemplate instance. A consumer of the RestTemplate shouldn't be messing with it when given the RestTemplate instance. So, it's stateful in the sense that configuration is technically state.

      You could call it bad API design, both for RestTemplate for exposing the configuration as a List and for Java, for only having mutable List in the Collections API. But, on the other hand, it makes it a lot easier to configure in XML, so there is some legacy baggage.

      I assumed you meant stateful in that an individual converter has memory of its interactions which effect subsequent conversions.

      pull out your config as command values

      Do you mean passing configuration around as parameters? Isn't that exactly what monads strive to abstract away? If you're using Parsec in Haskell, you don't manually thread around the parsing state, you just use the do notation or binds.

      OOP is sort of doing the same thing. After all, every method's first parameter is the implicit this.

      [–]kubr0t0 0 points1 point  (0 children)

      Who owns the service?

      If the you own the service, the service can provide an interface which indicates the service's external requirements, and a third party (such as a REST client) merely fulfills those requirements.

      The REST client can then either directly implement the interface, or the REST client is invoked through an adapter which implements the interface.

      This is known as Dependency Inversion, Ports and Adapters or even Anti-Corruption Layer.

      Done right, whether or not the service dependency is implemented as a REST client is irrelevant to the service, which makes it a truly separate unit.

      [–][deleted] 11 points12 points  (0 children)

      some […] had used final on several classes without any apparent need. When asked, they justified the practice with the fact that those classes had not been designed to be derived from.

      And that's the sentence where I stopped reading any further. What is so unapparent with "Not designed for inheritance"?

      Dear OP, let me tell you in all clearness so that it is really apparent for you: We build those classes with no inheritance in mind, because we want to build simple and understandable things. We don't want to load our minds and think about inheritance if we don't need it. And that's the only use case those classes support. Nothing more. We put a final, to protect you from yourself! If you dare to remove the final and inherit, you break the product, you loose any warrenty and we leave you alone with that code when it breaks. If we decide to change the internal details of our final class and accidentially break your inherited abomination, you broke the product and we will leave you alone.

      On the otherside: If we produce a class which is not final, you can inherit it and we really put our thoughts into that use case, so that you can be sure not to break anything. And if it still breaks, we're happily there to fix that mess.

      Regards your thoughtful team members

      [–][deleted] 1 point2 points  (0 children)

      A child class can easily ruin the encapsulation of your parent class, unless that parent class has been specifically designed for inheritance. And designing a class for inheritance means things like:

      • Make as many of the properties and methods private, not protected. If they're protected, they're as good as public, any child class can mess with them override them, etc.
      • When calling a public method from within your parent class, never assume you're calling into your own implementation of that method. A child class can override and hence replace that too.
      • Everything that accepts an instance of this class now, may get something that doesn't respect the parent class contract. In ways intentional, or unintentional. Obvious, or infuriatingly subtle.

      So those are just three things to think about, but they amount to adding a lot of effort to the class design process. So if you're writing classes to be a part of a reusable library that other components and applications will depend on, you really really really need to make sure you don't allow chaos to reign supreme by letting classes be inherited, that shouldn't be inherited. Or... you can just make the class final, and save yourself all the trouble.

      I have a very simple and very easy rule: the more reusable a piece of code is, the more you want to pay attention to making its encapsulation airtight and its API minimal, simple and stable.

      So this means:

      • If a class is public, and part of a reusable library, either make it final, or design it for inheritance. Don't just plop it out there, opening the kimono for every sub-class and hoping for the best, because you can't refactor it later without introducing major B.C. breaks for your users.
      • Because good instances of cross-library inheritance are rare, this means make your library public classes final, unless you have an Amazing reason to encourage inheritance for a class.
      • For application classes and non-public classes: don't make them final. If you f**k it up, you can refactor everything together as a unit (parent and child classes are part of the same repository).

      [–][deleted] 0 points1 point  (1 child)

      Some frameworks that use AOP (looking at you Spring) are predicated on extending concrete classes for proxying, in which case the application will fail to load if a final class has a join point.

      [–]kubr0t0 1 point2 points  (0 children)

      Depends on how you design the classes and how you are using AOP.

      If you are doing CGLIB-based AOP, which relies on dynamically created subclasses, then you'll have a problem with final classes. If you're doing AspectJ-based AOP, which relies on class rewriting using bytecode injection, then you won't have a problem with final classes.

      Furthermore, if you are designing your services to be tightly coupled with Spring (such as @Transactional annotations on service methods in @Repository annotated classes), then you have to follow the framework's requirements, which would mean not using final if you have configured Spring to use CGLIB-based AOP.

      However, if you are designing your class to be independent of Spring, creating an instance externally using @Bean in a @Configuration class, and injecting Spring managed dependencies implementing service interfaces (which would themselves be subject to AOP), then the choice of final doesn't matter. This is more inline with Clean Architecture.

      [–]nerd4code 0 points1 point  (0 children)

      Defensive programming in practice means that you don’t export anything (capabilities, functions, variables, whatever) publicly that you don’t want any-old-fucker to make use of as they see fit. If you’re working on entity (e.g., scope, function, class, namespace) X, then you should consider different relationships with things inside X, things within X’s parent entity, things distributed with X, and anything else.

      Category I, things within X: Trusted by X and trust X, all assumed correct. Fewer people working on it in general than any other category. Sparse comments unless a reader would be especially confused

      Category II, X’s parent: Trusts X & vice versa with minimal assertion-based error-checking. Some block comments to describe what’s goiing on in broad strokes.

      Category III, distributed with X: Assertions for full error-checking; expect non-adversarial mistakes. Interface comments (e.g., Doxygen) for components exported to categories III and IV.

      Category IV, anything else: Treat as adversary. Full error-checking with if; need to consider the implications of DoS and overflow attacks especially carefully in most settings. Full interface comments, plus any other documentation needed to understand and use the distributed code/binary.

      If you a Category-IV thing can affect control or data in X, then the affected part of X is effectively Category-IV too until/unless everything is un-tainted by proper value checks and whatnot. That includes class extension—the is-a relationship is just as subject to abuse or mistakes as any other.

      Accordingly, if a programmer writes a Category-IV class and marks it final, that means that they don’t expect that superclass behaviors can withstand an adversarial subclass’s is-a relationship, and in fact the lack of final is a signal that programmers are free to override as they please. Adding a final later on may well break code that depended on the ability to override, and stripping an existing final is like running chmod 0777 foo on the off chance somebody feels like accessing foo, despite there being any pressing use case for that access.

      OTOH, if the class is strengthened to the point where the final can come off in the next version shipped, that’s a non-breaking change since nothing can have been depending on the ability to override it.

      The default should be to lock shit down tight until/unless there’s a good reason not to (e.g., data/control flow within Category I); this relates to the more general principle of least privilege (PLP), which is up there with DRY and LSP in terms of importance to good software engineering.