Constructor Injection, design for testability

Question

I have this code (you probably can ignore that it is Swing code), but I usually end up with too many arguments in my constructor. Should I use model bean class and then pass that object in the constructor?

public BrowserFrame(final JTextField url, final JTextArea response, final JTextField command, final JButton actionButton) {

    this.urlField = url;
    this.responseArea = response;
    this.commandField = command;
    this.actionButton = actionButton;

}

In this code, I am considering adding more objects, used by this class. Do I keep just adding more args and pass them in the constructor. Possibly use setting injection?

But according to Misko, this is also code smell.

http://misko.hevery.com/code-reviewers-guide/

"Objects are passed in but never used directly (only used to get access to other objects)"

Answer 1

Since you want to clean-up Your code and improve its testability maybe You could go for setter initialization. This way You'd have a cleaner constructor for Your object. It is also easier to test code if you can set just "some" of its fields not all. Tests become more readable.

It's worth looking into Spring, because even if you're not using it, you could benefit from the guidelines of this framework.

Answer 2

BrowserFrame tends to indicate that you are extending a class (JFrame) where you don't need. That is bad.

I tend to have a layout layer about the JComponent/view layer.

class ThingView {
    [...]
    public JTextField /*or JComponent*/ createURL() { /* or geURL */
        return new JTextField(...); // Or from a field if using get.
    }
    [...]
}

class ThingLayout {
    private final ThingView view; 
    [...]
    public JPanel createBrowsePanel() {
        JPanel panel = new JPanel();
        [...]
        panel.add(view.createURL());
        [...]
        return panel;
    }
}

Answer 3

Having too many arguments in your constructor is a sign that your class has too many responsibilities. If you're just setting fields in your constructor, look to see if some of those fields are only used by a subset of the methods in the class. That's a sign that your arguments and methods can be split up into smaller classes with more focused responsibility.

Answer 4

Agreed with Bill that this may indicate too many responsibilities. But if you really can't pull those responsibilities out, Introduce Parameter Object is one way of cleaning up busy constructors/methods like that.

Answer 5

I don't think that we should ignore that this is swing code. GUIs aggregate several disparate objects in an organized way. I would advise you to seriously consider refactoring if you find yourself supporting multiple constructors, because it will be very confusing to understand what is and isn't necessary. But if you are putting seven components into some organizing template, it's not necessarily a bad idea to do that all in the constructor if it is unambiguous what each argument does.

If the question is "Does this class have too many concerns?" stop asking "What are the signs that a class has too many concerns?" Why not ask "What is this class's concern?" If the answer is something simple like "It organizes several form elements into a single view," which is what I (and by extension, other parties browsing your code) would logically assume, then stop worrying about cosmetics like constructors. It's obvious enough in this case. Adding more classes is just going to obfuscate that.

Answer 6

The question is framed as a creational/mechanical concern, but as many previous answers have pointed out, this is actually a design issue.

I think a reassessment of the object composition scheme might be more useful in the long run, such as realigning the abstraction so that a BrowserFrame is composed of Sections (e.g., AddressSection, ActionSection, InputSection, FeedbackSection, etc.) instead of individual Swing objects,

You could also refactor excessive parameterization of BrowserFrame construction by using sub-classing instead.