2

I have a class that has dependencies that I know are not going to change.

class ConversationFinder
{   
    public function __construct(
        protected Conversation $conversationDbFinder = new Conversation,
        protected ConversationMessages $conversationMessagesDbFinder = new ConversationMessages,
        protected QueuedMessages $queuedMessages = new QueuedMessages
    ){}
...
    }

In here $conversationDbFinder and $conversatoinMessagesDbFinder are concrete classes that are unlikely going to change. $conversationDbFinder will fetch the conversations list, and the $conversationMessagesDbFinder will fetch the messages for each conversation, and also helps attaching lastMessage and unreadMessageCount for each conversation. But even if they might, it's a big hassle to instantiate them everywhere before instantiating ConversationFinder and I believe it violates encapsulation. QueuedMessages is also something that is unlikely to change, I have kept it here to demonstrate that I know that I always have a DB and a Queue of messages (which will later get saved to the DB)

So the question is. Is that an okay design? Keep in mind that at the moment, the only reason why I'm including these params in the constructor is to later mock them in unit testing, otherwise the code will function the same if I just used them right away in the functions of the class.

3
  • 2
    Making classes unit-testable is probably the most popular use case for Dependency Injection, so why shouldn't it not be ok?`
    – Doc Brown
    Commented Jun 9, 2023 at 10:14
  • And your code will be more maintenable !
    – Adri
    Commented Jun 9, 2023 at 12:26
  • Dependency injection is a design technique. It can be useful even if you had no unit testing. It's like asking if functions are OK if they are called only once. Of course they are! By making some peace of code a function, you create a named code artifact, a "named thing". You clearly delineate the boundary of the (sub)problem, and the inputs and outputs, and this lets you express and solve the problem in isolation. Classes are no different, except that they give you an instance back. Constructor params communicate inputs and help isolate the (sub)problem. Commented Jun 11, 2023 at 9:29

2 Answers 2

9

It is absolutely okay to use a programming method only to support proper testing. Test suites are just as important as business code - the fact that you don't ship them to the customer is not important. The point of computer programming is to fulfill external requirements, and test code which aids that goal is a first-class citizen of the code base.

1
  • 1
    And, I would add, test mocks are a completely valid implementation of the abstract classes or interfaces used by your application. And when TDD is applied honestly, mocks and stubs are often your first concrete implementations of those abstractions. Commented Jun 9, 2023 at 12:24
1

I have a class that has dependencies that I know are not going to change.

No you don't. Even if you dedicate the rest of your life to making this prophecy come true, you still don't know. This is insisting and hoping.

In here $conversationDbFinder and $conversatoinMessagesDbFinder are concrete classes that are unlikely going to change.

Just means you have known good default values. So long as deploying this class and its hard coded default values together isn't a problem then this is fine. It has nothing to do with permitting overriding these known good default values.

What you're scratching around for is something called Convention over Configuration. The idea is to not sacrifice the simplicity of implicitly using obvious defaults for the sake of infinite configuration options.

At the class level the simplest version of this idea is a default constructor that initializes the default values paired with one that allows you to override those default values. Thus nothing is set in stone.

If the hard coding becomes an actual problem because of deployment issues hard coded defaults can be done in builders. Though that just moves the same problem to the builders.

One really nice thing about builders is you don't have to only have one set of known good defaults. Pack your defaults in builders and the limiting factor becomes coming up with names for the builders. A lot of expressive power to be gained here.

it's a big hassle to instantiate them everywhere before instantiating ConversationFinder and I believe it violates encapsulation.

Well no. That's not an encapsulation violation. That's just annoying. DI has a solution to this problem already. It's called the composition root==. Build it once. Pass it everywhere. No encapsulation violation since once you're handed this you don't have to tell no body bout it no how.

So the question is. Is that an okay design? Keep in mind that at the moment, the only reason why I'm including these params in the constructor is to later mock them in unit testing

That sounds silly. Include them if you use them. Testing shouldn't be that hard.

Is it okay to use Dependency injection only because of unit testing?

Dependency Injection is a fancy new term for a very old idea. Back in the day we just called this reference passing. Stop letting the books and frameworks make a bigger deal out of this than it deserves. You can build something and pass references to it around as much as you like.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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