|
KeepConstructorsHidden
Keep constructors on Guice-instantiated classes as hidden as possible.Consider this simple interface: public interface DataReader {
Data readData(DataSource dataSource);
}It's a common reflex to implement this interface with a public class: public class DatabaseDataReader implements DataReader {
private final ConnectionManager connectionManager;
@Inject
public DatabaseDataReader(
ConnectionManager connectionManager) {
this.connectionManager = connectionManager;
}
@Override
public Data readData(DataSource dataSource) {
// ... read data from the database
return Data.of(readInData, someMetaData);
}
}A quick inspection of this code reveals nothing faulty about this implementation. Unfortunately, such an inspection excludes the dimension of time and the inevitability of an unguarded code base to become more tightly coupled within itself over time. Similar to the old axiom, Nothing good happens after midnight, we also know that Nothing good happens after making a constructor public: A public constructor will have illicit uses introduced within a code base. These uses necessarily will:
Perhaps worst of all, any direct use of a constructor circumvents Guice's object instantiation. As a correction, simply limit the visibility of both your implementation classes, and their constructors. Typically package private is preferred for both, as this facilitates:
As a simple, mnemonic remember that public and @Inject are like Elves and Dwarfs: they can work together, but in an ideal world, they would coexist independently. | |
► Sign in to add a comment
You tell us what not to do but not what we should do? Private class? Private ctors?
Package local/private class is my preference. Or the same for (or protected) constructor depending on your use case. A lot of this depends on how the specific class in intended to be used.
The doc's main point is, prefer depending on interfaces and let Guice call new, as this gives you the maximum amount of control and flexibility of design.
Wouldn't making the ctor or class private or package protected make it impossible to test?
No, tests should be in the same package as the class.
Really bad explanation.
If you use a private constructor now your classes depends on the DI framework, they are more difficult to test: how do you write an unit test for DatabaseDataReader? without using Guice or other framework that supports the @Inject annotation? The answer is to put the unit tests in the same package, but sometimes you want to make unit test for other classes in the same project module (but in other package) that use the implementation, yes you can use mock objects but an abuse of them also makes unit test difficult to maintain.
As a simple, mnemonic remember that public and @Inject are like Elves and Dwarfs: they can work together, but in an ideal world, they would coexist independently.
LOL good luck when you want to use your implementation in other project that doesn't use Guice
Agreed with diego... alsmost on 99%, this topic isn't convince me as well.
I don't like idea to be restricted to keep module class (or provider) in same package as interface implementation, it not always make sence, I could have tons of interfaces and implementation in one project and I'll still have to pay same efforts during refactor, changing module or provider instead of class constructor.