Use constructor or setter method?

Question

I am working on a UI code where I have an Action class, something like this -

public class MyAction extends Action {
    public MyAction() {
        setText("My Action Text");
        setToolTip("My Action Tool tip");
        setImage("Some Image");
    }
}

When this Action class was created, it was pretty much assumed that the Action class won't be customizable (in a sense - its text, tooltip or image will be not be changed anywhere in the code). Now, we are in need of changing the action text at some location in code. So, I suggested my co-worker to remove the hardcoded action text from the constructor and accept it as an argument, so that everybody is forced to pass the action text. Something like this code below -

public class MyAction extends Action {
    public MyAction(String actionText) {
        setText(actionText);
        setTooltip("My Action tool tip"); 
        setImage("My Image"); 
    }
}

He, however, thinks that since setText() method belongs to base class it can be flexibly used to pass the action text wherever action instance is created. That way, there is no need to change the existing MyAction class. So his code would look something like this.

MyAction action = new MyAction(); //this creates action instance with the hardcoded text
action.setText("User required new action text"); //overwrite the existing text.

I am not sure if that is a correct way to deal with problem. I think in above mentioned case user is anyway going to change the text, so why not force him while constructing the action? The only benefit I see with the original code is that user can create Action class without much thinking about setting text.

Answer 1

The only benefit I see with the original code is that user can create Action class without much thinking about setting text.

That is actually not a benefit, for most purposes it's a drawback and in the remaining cases I'd call it a tie. What if someone forgets to call setText() after construction? What if that is the case in some unusual case, perhaps an error handler? If you want to really force the text to be set, you need to force it at compilation time, since only compile-time errors are actually fatal. Anything that happens at run-time depends on that particular code path being executed.

I see two clear paths forward:

1. Use a constructor parameter, as you suggest. If you really want to, you can pass null or an empty string, but then the fact that you are not assigning a text is explicit rather than implicit. It's easy to see the existence of a null parameter and see that there was probably some thought put into it, but not so easy to see the lack of a method call and determine if the lack of such was intentional or not. For a simple case like this, this is probably the approach I would take.
2. Use a factory/builder pattern. This may be overkill for such a simple scenario, but in a more general case it is highly flexible as it allows you to set any number of parameters, and check preconditions before or during object instantiation (if constructing the object is a large operation and/or the class can be used in more than one way, this can be a huge advantage). Particularly in Java it is also a common idiom, and following established patterns in the language and framework you are using is very rarely a bad thing.

Answer 2

Constructor overloading would be a simple and straightforward solution here:

public class MyAction extends Action {
    public MyAction(String actionText) {
        setText(actionText);
        setTooltip("My Action tool tip"); 
        setImage("My Image"); 
    }
    public MyAction() {
        this("My Action Text");
    }
}

It is better than calling .setText later, because this way nothing needs to be overwritten, actionText can be the intended thing right from the start.

As your code evolves and you will need even more flexibility (which will surely happen), you will benefit from the factory/builder pattern suggested by another answer.

Answer 3

Add a fluent 'setText' method:

public class MyAction ... {
  ...
  public MyAction setText(String text) { ... ; return this; }
}

MyAction a = new MyAction().setText("xxx");

What could be clearer than that? If you decide to add another customizable property, no problem.

Answer 4

Just like kevin cline said in his answer, I think the way to go is to create a fluent API. I would just like to add that the fluent API works better when you have more than one property you may use.

It will make your code more readable, and from my point of view more easy and, aham, "sexy" to write.

In your case it would go like this (sorry for any typo, it has been a year since I wrote my last java program):

 public class MyAction extends Action {
    private String _text     = "";
    private String _tooltip  = "";
    private String _imageUrl = "";

    public MyAction()
    {
       // nothing to do here.
    }

    public MyAction text(string value)
    {
       this._text = value;
       return this;
    }

    public MyAction tooltip(string value)
    {
       this._tooltip = value;
       return this;
    }

    public MyAction image(string value)
    {
       this._imageUrl = value;
       return this;
    }
}

And the usage would be like this:

MyAction action = new MyAction()
    .text("My Action Text")
    .tooltip("My Action Tool tip")
    .image("Some Image");

Answer 5

The advice to use constructors or builders is fine in general, but, in my experience, misses some key points for Actions, which

1. Possibly need to be internationalized
2. Are likely for marketing to change at the last minute.

I strongly suggest that the name, tooltip, icon etc... be read from a properties file, XML, etc. For example, for the File-Open action, you could pass in a Properties and it would look for

File.open.name=Open
File.open.tooltip=Open a file
File.open.icon=somedir/open.jpg

This is a format pretty easy to translate to French, to try a new better icon, etc. Without programmer time or a recompile.

This is just a rough outline, a lot is left to the reader... Look for other examples of internationization.

Answer 6

It is useless to call setText(actionText) or setTooltip("My action tool tip") inside the constructor; it is easier (and you gain more performance) if you simply initialize the corresponding field directly:

    public MyAction(String actionText) {
        this.actionText = actionText;
    }

If you change actionText during the life time of the MyAction corresponding object, you should place a setter method; if not initialize the field only in constructor without providing a setter method.

Since tooltip and image are constants, treat them as constants; have fields:

private (or even public) final static String TOOLTIP = "My Action Tooltip";

Actually, when designing common objects (not beans or objects representing strictly data structures) it is a bad idea to provide setters and getters, since they kind of break encapsulation.

Answer 7

I think this is true if we are going to make a generic action class (like update, which is used to update Employee, Department ...). It’s all depends on scenario. If a specific action (like update employee) class (used many place in application - Update employee) is created with the intension to keep same text, tooltip and image at every place in the application (for consistency point of view). So hardcoding can be done for text, tooltip and image to provide the default text, tooltip and image. Still to give more flexibility, to customize these, it should have corresponding setter methods. Keeping in the mind only 10% places we have to change it. Taking action text every time from user can cause Different text every time for same action. Like 'Update Emp', 'Update Employee', 'Change Employee' or 'Edit Employee'. I am not sure but I think this can create the confusion to the user that this is something different.

Answer 8

Think of how instances will be used and use a solution that will guide, or even force, the users to use those instances in the right, or at least best, manner. A programmer using this class will have a lot of other things to worry and think about. This class should not add to the list.

For instance, if the MyAction class is supposed to be immutable after construction (and possibly other initialization), it should not have a setter method. If most of the time it will use the default "My Action Text", there should be a parameterless constructor, plus a constructor that allows an optional text. Now the user doesn't need to think to use the class correctly 90% of the time. If the user usually ought to give some thought to the text, skip the parameterless constructor. Now the user is forced to think when necessary and can't overlook a necessary step.

If a MyAction instance needs to be mutable after full construction then you need a setter for the text. It's tempting to skip setting the value in the constructor (DRY principle--"Don't Repeat Yourself") and, if the default value is usually good enough I would. But if it's not, requiring the text in the constructor forces the user to think when they should.

Note that these users are not dumb. They just have too many real problems to worry about. By thinking about your class's "interface", you can keep it from becoming a real problem also--and an unnecessary one.

Answer 9

In the following proposed solution, superclass is abstract and has all three members set to a default value.

Subclass has different constructors so the programmer can instantiate it.

If first constructor is used, all members will have the default values.

If second constructor is used, you give an initial value to actionText member leaving other two members with the default value...

If third constructor is used, you instantiate it with a new value for actionText and toolTip, leaving imageURl whith the default value...

And so on.

public abstract class Action {
    protected String text = "Default action text";
    protected String toolTip = "Default action tool tip";
    protected String imageURl = "http://myserver.com/images/default.png";

    .... rest of code, I guess setters and getters
}

public class MyAction extends Action {


    public MyAction() {

    }

    public MyAction(String actionText) {
        setText(actionText);
    }

    public MyAction(String actionText, String toolTip_) {
        setText(actionText);
        setToolTip(toolTip_);   
    }

    public MyAction(String actionText, String toolTip_; String imageURL_) {
        setText(actionText);
        setToolTip(toolTip_);
        setImageURL(imageURL_);
    }


}