The following scenario should be familiar to most developers: You look for a utility function in an existing project. Quickly you find it and realize it is private, so you can’t use it in your class. Assuming you won’t copy-paste the function, a refactor is required. Extracting the function to a new designated class seems like the best solution.
One question remains — how will you extract the function?
The first option is a static function in a new class. The second option, assuming you use any dependency injection framework (e.g. Guice), is an injectable instance of a new, non-static class. In this post, we will explore the considerations of choosing between the two options.
NOTE: This article mainly refers to cases where the function is used as-is, without introducing any new abstractions.
TL;DR
Before choosing a solution, we should ask ourselves the following questions:
- Does the function have injectable dependencies (by the DI framework)?
- Will you need to modify the function’s behaviour in unit tests (e.g. by mocking)?
- Is it an impure function?
If the answer to any of the questions is yes, dependency injection (DI) is probably a better choice. Otherwise, we should replace with a static function.
Static Function by Default
By default, a static function would be a better solution.
Why?
- Code readability: A static reference to a function is usually easier to understand than an injected dependency.
- Code impact: Extracting an existing utility function to a new static function requires no further actions. Yet, using DI requires updating the constructor, adding new class field, and updating the existing tests.
- Testability and performance: It usually doesn’t matter if the function is static or injected.
There are some scenarios, however, where DI is preferred.
1. Impure function
Impure functions have side-effects. They can potentially raise issues like unsafe concurrency or race conditions.
Consider the following simple example:
You would like to refactor getNextNum, since you found out that multiple classes have implemented it in the same way. Perhaps lazy developers have copied the code without bothering to refactor it properly. You, however, are not a lazy developer. You want to refactor this function to reduce code duplication.
Quickly, you implement the following solution:
It takes a few seconds, but you realize that this implementation is not good enough.
The main problem is that NumIncrementor.num is now a shared resource. All classes that use NumIncrementor are now affecting one another. A second problem — NumIncrementor.logIncrement is not thread-safe. Calling it from multiple threads may cause unpredictable behaviour.
You realize that updating NumIncrementor to have non-static fields and using a new instance of NumIncrementor with each consumer would be the best approach.
For the example above, using Google’s Guice can help us. By default, Guice creates a new instance for injected classes (unless annotated with the @Singleton annotation). This is exactly the behaviour we want.
Let’s see how to configure Guice to inject a new instance of the utility class, instead of using a static class:
Each consumer instance will receive a new instance of the utility class when created. This way, NumIncrementor.num will no longer be shared with multiple dependants and threads.
All of the problems that were mentioned earlier will be solved, without a great impact on the code or the application.
Naturally, configurations for dependency injections are specific for each application and usage. The example above shows that in some cases, static references create issues that can be solved by using a properly configured DI.
It is important to mention that in some cases it is better to update code to be thread-safe. For example, by receiving a parameter instead of referencing to an object, or using thread-safe objects. However, this means changing the code behaviour, which we try to avoid in this post.
Bottom line: Look for resource-sensitive operations or impure functions. Check for problems like thread-safety and shared resources. When encountering such cases, consider using a properly configured DI for solving these problems.
2. Modifying the Behaviour of the Extracted Function in Unit Tests
Sometimes when writing unit tests, it’s useful to replace some of the tested unit’s dependencies with new behaviour. For example, when a dependency has many edge cases or has unwanted side-effects for tests. Using the original implementation in unit tests can lead to complex, difficult, and sometimes even incorrect tests.
Replacing the implementation of dependencies in unit tests has many advantages — isolation, shorter test duration, and easy elimination of side effects are just some of the profits. It can be done by one of two main approaches: mocking or abstracted interfaces.
Mocking is a useful testing concept. It allows you to replace the implementation of a certain object. Some find mocking a controversial topic, and advocate for using interfaces instead.
Either way, both approaches are extremely helpful in unit tests and go hand-in-hand with DI.
Contrary to DI, replacing a static class function for unit tests is difficult. It requires non-conventional tools. That’s why, when extracting a function that should be replaced in tests, prefer DI over static functions.
Bottom line: We will use DI when replacing the implementation of the extracted function improves your unit tests.
3. Injectable Parameters
Sometimes, a utility function uses dependencies applied to fields of the containing class. When extracting it to be static, the function loses access to those dependencies.
We can solve that by passing more arguments to the function, but that makes it less reusable and pollutes the consuming classes.
A preferred solution is extracting the function to a class which can be injected with DI. That way, the extracted function can access the same dependencies directly from the DI.
Consider the following example:
SomeClass holds a utility function, decryptToJson, which should be extracted to a new class, and depends on two parameters (input, beginIndex) in addition to two internal class fields which were injected by Guice (decryptor, jsonParser).
By extracting decryptToJson to a static function, we will no longer be able to use Guice for accessing dependencies inside the function. We will have to pass all the dependencies to the static function:
This forces the consuming classes to contain fields which are specific to the utility function decryptToJson. In our case, the fields are decryptor and jsonParser.
The utility function, on the other hand, now requires more parameters — four instead of the original two, which makes the code cumbersome and less readable, especially in complex cases with more parameters.
In cases such as the one above, using the DI framework to inject an instance of the extracted class will simplify the code:
Each dependency is now injected where it should be. The utility function no longer demands specific services as parameters. It only provides functionality for the consumers.
Notice how the code becomes more readable and less complex. It is important to note that testability and performance remain unharmed.
Bottom line: We will use DI when the extracted function requires parameters that can be injected through DI. This is preferable to passing the dependencies as parameters to a static function.
Conclusion
In this article, we’ve reviewed most of the things we should be paying attention to when refactoring an existing utility function.
Naturally, there is no single answer for all cases. Some scenarios require further considerations, however, you may refer to this post as a guideline — a rule-of-thumb — in case you are unsure of the best solution for refactoring your utility function.
Whatever you do, don’t copy-paste your function!
Further Reading
This post is part of Innovid’s technical blog.
Innovid is the only independent omni-channel advertising and analytics platform built for television.
