Bloated constructor means procedural code

Today, I was browsing through a codebase I’m currently working in and I’ve ran into a class with bunch of dependencies (to be precise, exactly 10 of them).
When I find myself in these kind of situations, I use my own kind of heuristics around figuring out which candidates are for refactoring (which I’ll talk about in some other post) and one of the rules is that if a class has more than 3 dependencies, that should be treated as a code smell and reconsidered from design perspective.
Usually, when having this many dependencies, my first guess is that given class exhibits low cohesion design smell, where some dependencies are used in some methods (at worst each dependency is used in one of the methods). On the contrary, if you use majority of your dependencies in majority of your methods, you have high cohesion.

Also, this usually means that class violates Single Responsibility Principle and is addressed by splitting up given class into different classes with clearly defined responsibilities. In that case, constructors of the separate classes now use only a subset of the original set of dependencies. Furthermore, this has positive ripple effect on the original client classes of original class, since that usually means that they can be split up as well.

So, while looking at the given class and how the dependencies were actually used throughout method implementations, I ran into a method with bunch of code that, when you Clint squint, actually see something like this.

As you can notice, this method uses bunch of dependencies (some are explicitly injected through constructor and some implicit through private methods) and thus, someone could say, has high cohesion. But, when you look carefully at the names of the methods it uses, you figure out that bunch of them start with ‘GetXYZ‘.

“Procedural code gets information than makes decisions, object oriented code tells objects to do things”
Alec Sharp

This means:

  • pure procedural code that mixes different levels of abstractions
  • heavy violation of Tell Don’t Ask principle
  • missing domain concept not represented as a layer of indirection between client class and dependencies

Also, there is a huge negative side effect when you think about unit tests and number of dependencies you need to stub in order to test the given method! From my experience, these are kind of methods that demotivate some developers to practice TDD saying that TDD is too hard to do, time consuming and introduces lot of friction in development.

On the contrary. Use design feedback provided by tests and fix your design. You’ll decrease coupling between classes, have higher cohesion and create code that communicates in domain terms.

Leave a Reply

Your email address will not be published. Required fields are marked *