0

I have been reading about inversion of control and dependency injection and I was wondering the following.

Is there a good way to tell when it is okay to initialise an object inside a method body of a class and when it's better to pass it as a constructor dependency?

For instance I was doing refactoring after finishing a TDD kata about generating greetings with some processing of some arguments. The code had some intermediate results that I used to construct the final string.

During refactoring I extracted some of the logic in their own classes by creating an instance at the point in the method where I needed them and passing the intermediate structures in their constructor and then call the method that operated on them.

Are there considerations in choosing to write a class that has one method that acts on its parameters vs writing a class that instead takes the same parameters in the constructor and the methods acts on its fields?

Here is what the code looked like after the first refactor

public class GreeterGenerator {
    private static final Greeter NO_NAME_GREETER = () -> "Hello, my friend."
    private final NameProcessor nameProcessor;

    public GreeterGenerator(NameProcessor nameProcessor) {
        this.nameProcessor = nameProcessor;
    }

    public String buildGreeting(String name) {
        return isEmpty(name)
                ? NO_NAME_GREETER.buildGreeting()
                : buildGreeting(new String[]{name});

    }

    private boolean isEmpty(String name) {
        return name == null || name.isEmpty();
    }

    public String buildGreeting(String[] names) {
        String[] processedNames = nameProcessor.splitAnyCommaSeparatedEntriesToSingleNames(names);

        var nameDifferentiator = new NameFilter(processedNames);

        var normalGreeter = new NormalGreeter(nameDifferentiator.filterNormalNames());
        var shoutingGreeter = new ShoutingGreeter(nameDifferentiator.filterShoutedNames());

        return new GreeterJoiner(normalGreeter, shoutingGreeter).joinGreetings();
    }
}

and this is after trying I moved the helper classes to constructor dependencies. The filtered names is just a wrapper around the two arrays that resulted from the methods in nameDifferentiator being called. The normalGreeter and shoutingGreeter were moved into the GreeterJoiner as constructor dependencies

public class GreeterGenerator {
    private static final String NO_NAME_GREETING = "Hello, my friend.";
    private final NameProcessor nameProcessor;
    private final NameFilter nameFilter;
    private final GreeterJoiner greeterJoiner;

    public GreeterGenerator(NameProcessor nameProcessor,
                            NameFilter nameFilter,
                            GreeterJoiner greeterJoiner) {
        this.nameProcessor = nameProcessor;
        this.nameFilter = nameFilter;
        this.greeterJoiner = greeterJoiner;
    }

    public String buildGreeting(String name) {
        return isEmpty(name)
                ? NO_NAME_GREETING
                : buildGreeting(new String[]{name});
    }

    private boolean isEmpty(String name) {
        return name == null || name.isEmpty();
    }

    public String buildGreeting(String[] names) {
        String[] processedNames = nameProcessor.splitAnyCommaSeparatedEntriesToSingleNames(names);

        var filteredNames = nameFilter.filterNames(processedNames);

        return greeterJoiner.joinGreetings(filteredNames);
    }
}

The code and commits (just filter out Greeter) can be seen here --> https://github.com/PhoenixRe32/tdd-exercises/tree/master/greeter It's a repo where I do my TDD katas as an exersice/refresher/or just pass the time (equivalent of doing sudoku I guess)

2 Answers 2

5

Are there considerations in choosing to write a class that has one method that acts on its parameters vs writing a class that instead takes the same parameters in the constructor and the methods acts on its fields?

There are two main reasons you'd abstract it away in a class:

  • Reusability, i.e. you want to use this sublogic in other classes as well
  • Cleanliness, i.e. the sublogic is sizeable and becomes a distraction to the parent class' other code.

Is there a good way to tell when it is okay to initialise an object inside a method body of a class and when it's better to pass it as a constructor dependency?

Use it as a constructor dependency when:

  • There are multiple possible implementations for this abstraction
  • You want the ability to switch implementation without changing the parent class, i.e. OCP-friendliness

Use direct initialization if:

  • The subclass is considered a private implementation detail
  • The subclass is a data container and has no meaningful behavior
  • Consumers of the parent class have no vested interest in what the subobject is, that it is used, or how it is used.

This is a careful balance and not easy to enshrine into a universal mantra. A simple data struct can clearly be instantiated, anything you'd call a "service" should clearly be a constructor dependency. Inbetween those two, the lines get more blurry and people find arguments either way.

In general, I'd err towards constructor dependencies, unless you have a strong argument as to why it's not relevant.

a class that instead takes the same parameters in the constructor and the methods acts on its fields?

What's particular about your scenario is that you're effectively creating a helper method, which is generally a static method, but you've object for an object-oriented approach, and I suspect the object constructor works a lot like the static method body would.

This isn't wrong per se, but it's something I've heard senior developers disagree on.

  • Some argue that static helpers "skip" the whole DI conversation and are considered as hardcoded data operations.
  • Some argue that static helpers are dirty and should be converted to instanced services which do get injected. The main argument I've heard here is the core DI argument: avoid hardcoding at all costs.

I think there' a line to be drawn here in what constitutes a helper method that is worth converting to an instanced service. I wouldn't particularly ban all static logic, but I would try to inject things that are less than trivial.

var nameDifferentiator = new NameFilter(processedNames);

var normalGreeter = new NormalGreeter(nameDifferentiator.filterNormalNames());
var shoutingGreeter = new ShoutingGreeter(nameDifferentiator.filterShoutedNames());

return new GreeterJoiner(normalGreeter, shoutingGreeter).joinGreetings();

In your case, I'd even consider injecting a GreetingFactory which generates the needed Greeting objects based on the String[] processedNames.

Assuming your Greeter classes derive from a base class, I would've put the join logic in that base class, rather than in a separate GreeterJoiner class.

You didn't specify the contents, but something like:

Edit: I noticed too late that you're using Java and this example is C#, but I assume the intent of the example is readable.

public class Greeter
{
    public readonly IEnumerable<string> Names = new List<string>();

    public virtual void AddName(string name)
    {
        this.Names.Add(name);
    }

    public Greeter Join(Greeter otherGreeter)
    {
        var mergedGreeter = new Greeter();

        foreach(var name in this.Names)
            mergedGreeter.Add(name);

        foreach(var name in otherGreeter.Names)
            mergedGreeter.Add(name);

        return mergedGreeter;
    }
} 

public class WhisperingGreeter : Greeter
{
    public override void Add(string name)
    {
        this.Names.Add(name.ToLower());
    }
}

public class ShoutingGreeter : Greeter
{
    public override void Add(string name)
    {
        this.Names.Add(name.ToUpper());
    }
}
6
  • Thank you @Flater for your insight. I always struggle to find people to have this random quick thoughts/conversations that crop up when one does a kata :-P What you are saying aligns with some thoughts I had. I remember that I felt that the joiner didn't quite agree with me and wanted to change how it was incorporated but I failed to follow through. The factory example you presented is quite nice and I will play with it a bit. C# is fine, it's basically Microsoft Java (:-P) and I have worked with .NET as well in my career (though not as much as JVM) Mar 1, 2021 at 14:17
  • On your comment on static logic; I am of similar mind, won't immediately ban all static logic but will always second guess it. So would you say that my second refactoring would be err'ing towards constructor dependencies? I find it hard to determine what is trivial and what not. The helper classes logic here is quite specific to the problem. Granted this is a Kata but even in a bigger system I would probably think that this is specific to this and would wait until I see a need to reuse it and then extract it to a constructor dependency. Mar 1, 2021 at 14:28
  • @AndreasAndreou "I find it hard to determine what is trivial and what not" It's a C# example again, but a good example of trivality is Path.Combine. It essentially concatenates incomplete filepaths and checks for absent/duplicate / characters inbetween. That's not meaningful enough to be considered an injectable dependency.
    – Flater
    Mar 1, 2021 at 15:05
  • Just a comment here, but: "I hate(!) 'dependency injection!'" 😡 Why? Because it makes it much more difficult to see what the source code is actually doing. There is "an injector," somewhere else, that is now having a very-material influence upon "what this code is doing when it runs." When I examine a (of course, unfamiliar to me ...) piece of code, I want to immediately see everything that has anything to do with what this code is actually going to do. Mar 1, 2021 at 16:20
  • 2
    @MikeRobinson: Reliance on coupling is an indication of bad development practices. It's no different from a micromanager refusing to stop micromanaging because they can't see what their staff are doing at every time of day. Much like I'd tell that micromanager and I'd tell you about wanting explicit coupling, not doing that is precisly the point. It's misdirected effort because of a false sense of needing to control it. Any IDE worth its salt can get you from one place to the next in two clicks ("find usages"/"find references")
    – Flater
    Mar 1, 2021 at 20:44
0

It depends on the type of object.

If you find yourself passing the same interface to each and every method of the object after you create it, you probably want to pass the interface to the constructor instead. There is obviously a strong relationship between the object and that interface, the object has no meaningful application without it.

If you have seven methods and only one of them depends on the interface, you want to pass it to that one method to keep your scope tight. The interface is not tied to the object, it is just tied to that one operation. So it has no place as a member of the object.

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.