Java: refactoring logic & impure functions for unit-tests
This is a small Java Coding Dojo where we analyze a piece of code given to us. Our mission is to unit-test it.
You can find all of the code from this article in its git repository.
1. The code
Here is the code we are given:
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
public final class Base {
public static String readOrDefault(final String path) {
final var file = new File(path);
if (!file.exists()) {
return "Default";
}
try {
return Files.readString(Path.of(path));
} catch (IOException e) {
return "Default";
}
}
}
2. Is testing this code a good idea?
This piece of code is in charge of multiple things:
-
Checking that a file exists;
-
Read content of a file as string;
-
Return a default value in some cases.
2.1. Why can’t it be unit-tested?
1) and 2) are side-effects, because they read a file system. Furthermore, they are buried inside the method, and we have no access to them.
This means that we cannot control them, and thus it cannot be unit-tested.
However, this code can be tested with an integration test.
2.2. Mix of concerns
3) clearly is some kind of logic. Should this logic be tightly coupled with reading a file from the file system? Very unlikely.
2.3. Architecture issue
Separating the reading of file content from the logic is the driver towards better testability.
The Hexagonal/Ports & Adapters architecture is a good way to see things, basically: push side-effects to the outer-edge of your application, and keep the code handling the logic only.
As a matter of fact, as a rule of thumb, we can separate things very well:
-
business logic only appears in pure functions;
-
side-effects (aka impure functions) are the dumbest possible.
3. How to refactor it
As we’ve now identified 3 things this function does, we should try to separate them from each other.
Usually, Java developers tend to group the side effects together, so grouping 1) and 2).
To become testable, 1) and 2) need to become a dependency for this function.
To do that, we first extract those 2 side-effects into its own interface, so that we can provide our own implementation for testing purposes:
public interface DataHandling {
boolean exists(final String path);
String readString(final String path);
}
Then, we can provide this contract as a dependency to the base code. It’s customary to do so via the constructor of the class in Java:
public final class BaseRefactored {
private final DataHandling handler;
public BaseRefactored(final DataHandling handler) {
this.handler = handler;
}
public String readOrDefault(final String path) {
if (this.handler.exists(path)) {
return "Default";
}
return this.handler.readString(path);
}
}
We are now free to provide a handler
to test it.
Eventually, we also need to provide the implementation of this handler
for file systems:
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
public class FileHandling implements DataHandling {
@Override
public boolean exists(String path) {
return new File(path).exists();
}
@Override
public String readString(String path) {
try {
return Files.readString(Path.of(path));
} catch (IOException e) {
return "Default";
}
}
}
4. Cost of this refactoring
However, we modified the signature of our base function, as it no longer is static
.
Actually, in Java, static
methods probably should always be pure functions.
5. Any other solutions?
This isn’t the only solution, and the repository itself provides more solutions to this problem, each with pros and cons.
I suggest you have a look at the others solutions, or try to come up with new ones. Feel free to contribute to the code as well.