Limits of dependency injection

Question

I use dependency injection with Guice and most of the time, I'm rather happy with it. But sometimes, it's difficult to get the dependencies to where they're needed and sometimes, it's plain impossible.

An extreme example is making MyResponse.toString() return some JSON, where MyResponse contains either a list of DTOs or an Exception. There are requirements, that the stacktrace should only be shown to testers, which means that some information about the logged-in user needs to be provided somehow. Additionally, when testing locally, I obviously always want to see the stacktrace, even when logged-out. And there are more such rules.

Obviously, this dependency can be included in MyResponse or provided as arguments (though not to the very toString() method, but I agree that toString() should usually be used for debugging only). This gets pretty ugly when a new such dependency gets added. For example, I just had to add a localized end-user message to the JSON representation of the exception. The number of such second-class dependencies grew slowly over the number of dependencies relevant for the core functionality.

Sometimes, they can be aggregated nicely, e.g., I can provide a properly configured Gson, which takes care about the conditional stacktrace hiding and the localized end-user message generation.(*) But oftentimes, it doesn't work well and I end up passing lots of arguments, or passing needlessly big dependency aggregates, or creating single-use aggregates. In the end, it feels like I'm overusing DI as it starts to make the code more ugly rather then nicer.

I wonder, if there's a point where I should simply give up and say "these boring details will be available from a thread-local (or even a singleton)"? I don't think it'd hamper testing as these dependencies hardly ever matter and when they do, they can be set as needed.

---

(*) This may sound like putting too many responsibilities to the poor Gson instance, however, it's the thing responsible for converting objects to JSON, so it should do it exactly like needed. The Gson is composed from a bunch of adapters and making the ThrowableAdapter be aware of the stacktrace hiding and localized user-message generation sounded like a good idea. The only problem is that the needed information (1. Should the stacktrace be shown? 2. What's the language for the message?) has to be passed somehow to it.

Thinking about it more, I could wrap the Throwable into MyThrowableWithUserInfo and write an adapter for it (I can always do this wrapping before needing Gson). In this particular case it seems to be the perfect DI solution.

In general, it's not that good as there could be throwables somewhere deep in the to be serialized object and adding this information to them may be impossible. I'm afraid, it'd need a thread-local hack, but this hack can be constrained to the serialization process only, which makes it much less ugly.

Answer 1

Analyzing your problem:

... sometimes, it's difficult to get the dependencies to where they're needed and sometimes, it's plain impossible.

... An extreme example is making MyResponse.toString() return some JSON...

... stacktrace should only be shown to testers ...

... Obviously, this dependency can be included in MyResponse or provided as arguments...

From your context, it seems to me that Dependency Injection itself might not be the core problem. Instead, IMO maybe the design of your classes are getting too complex, being difficult to handle dependencies and relationships. Also, I guess you should review your overall structure of classes and check if your application is following all best practices and principles. For instance:

I can provide a properly configured Gson, which takes care about the conditional stacktrace hiding and the localized end-user message generation.

It seems that this object would be violating the single responsibility principle. By doing stuff like this, you're negatively impacting your application design, making it difficult for further maintenance.

So, taking a look at your problem:

But oftentimes, it doesn't work well and I end up passing lots of arguments, or passing needlessly big dependency aggregates, or creating single-use aggregates. In the end, it feels like I'm overusing DI as it starts to make the code more ugly rather then nicer.

I suggest that you review your application design, and try to make use of design patterns that can help handling dependencies or abstractions. Eg: you could try to use:

• Facade pattern: client calls the service, which hides a more complex Api, aggregating or managing parts of the response;
• Mediator pattern: objects that simplify relationships between lots of objects that might work together for some task;
• Decorator pattern: dynamically add behavior to objects at runtime;
• Review all classes and modules and make sure they're not violating other principles of solid as well;

For now I can think of these patterns, if I have more ideas I'll edit my answer.

Facade: https://stackoverflow.com/questions/5242429/what-is-facade-design-pattern

Mediator: https://en.wikipedia.org/wiki/Mediator_pattern

Decorator: https://en.wikipedia.org/wiki/Decorator_pattern

---

After reading your edits, I suggest the Decorator pattern for your case. Below I provide some code examples based on response examples.

class MyResponse {
    //list of Dtos or exception
}

interface MyResponseProcessor {
    MyResponse processResponse(MyResponse originalResponse);
}

interface MyResponseProcessorDecorator implements MyResponseProcessor {
    private MyResponseProcessor processor;
    
    MyResponse processResponse(MyResponse originalResponse);
}

class MyResponseBaseProcessor implements MyResponseProcessor {

    MyResponse processResponse(MyResponse originalResponse) {
        //basic processing of your response information
    }
}

class MyResponseStackTraceProcessor implements MyResponseProcessorDecorator {
    private MyResponseProcessor processor;
    
    public MyResponseStackTraceProcessor(MyResponseProcessor processor) {
        this.processor = processor;
        //Maybe inject some object capable of retrieving current user's permissions,
        //in order to know if should hide stack trace or not
    }
    
    MyResponse processResponse(MyResponse originalResponse) {
        //transforms original response, by removing info
        //depending on user's role of access
        //Returns a new response with the transformed information
    }
}

class MyResponseLocalizedErrorMessageProcessor implements MyResponseProcessorDecorator {
    private MyResponseProcessor processor;
    
    public MyResponseStackTraceProcessor(MyResponseProcessor processor) {
        this.processor = processor;
        //Inject other objects capable of loading current localization info
    }
    
    MyResponse processResponse(MyResponse originalResponse) {
        //transforms original response, by loading appropriate
        //localized string according to the error message within the 
        //original response;
        //Returns new MyResponse with error message added;
    }
}
 
interface MyResponseSerializer {
    String serialize(MyResponse response);
}
 
class MyResponseJsonSerializer implements MyResponseSerializer {
    String serialize(MyResponse response) {
        //only gets the info and convert them to Json
    }
}
 
class MyResponseXmlSerializer implements MyResponseSerializer {
    String serialize(MyResponse response) {
        //only gets the info and convert them to Xml
    }
}


//Example of use

String getSerializedResponse(MyResponse response) {
    
    //Decorations for response processor
    MyResponseProcessor processor = new MyResponseBaseProcessor();
    processor = new MyResponseStackTraceProcessor(processor);
    processor = new MyResponseLocalizedErrorMessageProcessor(processor);

    MyResponse processedResponse = processor.processResponse(response);
    
    MyResponseSerializer serializer = new MyResponseJsonSerializer();
    
    return serializer.serialize(processedResponse);
}