I am a Java expert with many years of developing applications and systems. I love learning new stuff and passing whatever I know to others. I love clean code, TDD and the simplest design possible. Eyal is a DZone MVB and is not an employee of DZone and has posted 23 posts at DZone. You can read more from them at their website. View Full User Profile

The Abuse of Utility Creational Methods

11.20.2009
| 4472 views |
  • submit to reddit

Sometimes I encounter code, which obviously the writer had good intentions in making the development easier and faster. However, in my opinion, it is not. It’s a kind of code that magically creates new instances and attaches one to another.

Usually it will be a static method in a utility class that accepts many parameters, probably Boolean flags among them. A method that in order to understand what exactly it is doing, you’ll need to drill down inside it and check what each parameter does. And to maintain the comments (if there are are any) of this API will probably be a nightmare.

I will show an example of what I mean using Apache Wicket. I’ve been developing an application with Wicket for more than a year now. Wicket resembles Java Swing in many aspects. The concept of a Page in Wicket resembles Java Swing’s JFrame. The concept of a Component that is added to another one is also similar. Below is a code of adding a TextField and a Label to a Form and attaching the Label to the TextField. We want to “attach” the label to the text field for error messages in a Feedback messages (among other things).

TextField patternTextField = new TextField("pattern");
patternTextField.setRequired(true);
patternTextField.setLabel(new ResourceModel("patternKey"));
add(patternTextField);

Below is the code that uses a utility method that does the same thing.

public static FormComponentLabel setFieldWithLabel(MarkupContainer container,String id,FormComponent formComponent,String labelKey,boolean setRequired) {
return setFieldWithLabel(container, id, formComponent, new ResourceModel(labelKey), setRequired);
}

Looking at that line, it’s not clear what exactly is done. In order to understand, below is the drill down of that method.

public static FormComponentLabel setFieldWithLabel(MarkupContainer container,String id,FormComponent formComponent,IModel labelKeyModel,boolean setRequired) {
formComponent.setLabel(labelKeyModel);
if (setRequired) {
formComponent.setRequired(true);
}
FormComponentLabel label = new CAFormComponentLabel(id, formComponent);
container.add(label);
container.add(formComponent);
return label;
}

 Take a look at another sequence of factory methods that do much more on the components.

public static void addFormLabelAndField(MarkupContainer container, String wicketID,String localizationID, boolean required,IValidator... validators) {
addFormLabelAndField(container, wicketID, localizationID, required, true,validators);
}

public static void addFormLabelAndField(MarkupContainer container, String wicketID,String localizationID, boolean required,boolean isVisable,IValidator... validators) {
addFormLabelAndField(container, wicketID, localizationID, required, isVisable, true,null, validators);
}

public static void addFormLabelAndField(MarkupContainer container, String wicketID,String localizationID, boolean required,boolean isVisable,boolean isEnable,AjaxEventBehavior behavior,IValidator... validators) {
addFormComponentsAndErrorFiels(container, new CATextField(wicketID), LabelType.CAFormComponentLabel, wicketID, localizationID, required, isVisable, isEnable, behavior, validators);
}

public static void addFormComponentsAndErrorFiels(MarkupContainer container,FormComponent firstComponent,LabelType labelType, String wicketID,String localizationID, boolean required,boolean isVisable,boolean isEnable,AjaxEventBehavior behavior,IValidator... validators) {
addFormComponentsAndErrorFiels(container, firstComponent, labelType, wicketID, localizationID, required, isVisable, isEnable, behavior,null, validators);
}

public static void addFormComponentsAndErrorFiels(MarkupContainer container,FormComponent firstComponent,LabelType labelType, String wicketID,String localizationID, boolean required,boolean isVisable,boolean isEnable,AjaxEventBehavior behavior,FormComponent labelComponent,IValidator... validators) {
for (IValidator validator : validators) {
firstComponent.add(validator);
}
if (behavior != null){
firstComponent.add(behavior);
}
firstComponent.setRequired(required);
firstComponent.setLabel(new ResourceModel(localizationID));
firstComponent.setEnabled(isEnable);
Component label = null;
switch (labelType) {
case CheckboxLabel:
label = new CheckboxLabel(wicketID+"Lbl", new ResourceModel(localizationID));
break;
case FormComponentLabel:
default:
label = new FormComponentLabel(wicketID+"Lbl", (labelComponent == null) ? firstComponent:labelComponent);
break;
}
label.setVisible(isVisable);
firstComponent.setVisible(isVisable);
container.add(label);
container.add(firstComponent);
}

It’s obvious that the original code is much clearer to read than the one with the utility method. On the other side, it looks as it is easier / faster to use the utility method. So which is better? Although there must be situations that the “utility way” is better, I strongly believe that the “normal way” should be preferred most of the times (after rereading, I strongly believe that it should be preferred ALL THE TIME).

Why? The answer is readability. I think that one of the reasons of writing “the utility way” code is that the developer “doesn’t think ahead”. What I mean by that is that instead of thinking how the code will be easy to understand in a few months from now (readability of code), the developer thinks how to make the programming “of now” faster (let’s write just one line instead of three).

Whenever I write code I keep in mind two things specifically:

  1. Make the code work and test it (which is obvious)
  2. “I write the code to someone else. A person whom I will never meet”

Let me elaborate about number 2. Here’s a quote from Martin Fowler’s Refactoring book: “Any fool can write code that a computer can understand. Good programmers write code that humans can understand.” I’m not saying that I’m a good (nor bad) programmer. All I am saying is that this phrase is in my mind whenever I write code. So, when I write code, I think about the person who will replace me some time in the future. A person that will probably be unfortunate to debug my code without me around to explain to him what’s going on.

So, looking back at the example, my opinion is that although the “utility way code” does what it is supposed to be doing, and may even helps to code faster without tedious repetitions, it is not better. I think that it is worse.

Published at DZone with permission of Eyal Golan, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Marcell Barbacena replied on Fri, 2009/11/20 - 12:15pm

Unfortunally I have to disagree with the argument but I agree that the produced code as is showed is bad.

I developed the same as:

form.add(labeled(textField(id, modelClass, validate)));

For that is necessary a convention on label as id+"Label".

The modelClass is optional and the validade is to set required field and to plug in the bean validation on the field.

Frank Silbermann replied on Fri, 2009/11/20 - 3:04pm

If you call one long method with lots of parameters instead of three short method calls, you're not reducing your code smaller -- you're just making it squatter (shorter but wider). That's like putting multiple statements separated by semicolons on a single line -- but even worse, you also have complex method definitions to read.

Now, if you were making a great many identical choices when instantiating a set of components, you could write a utility method with a very short and simple interface -- then you'd be replacing each repetitive group of property settings calls with a single short method call. That might make the code easier to read -- by preventing repetitive boilerplate from obscuring the interesting parts.

Making these decisions is sort of like the hand-optimization that assembly programmers would consider -- only instead of minimizing the computer's memory and processor cycles, you're trying to minimize the burden on the reader's memory and the number of logical reasoning steps the reader must go through in his mind.

Eyal Golan replied on Sun, 2009/11/22 - 9:44am in response to: Marcell Barbacena

My argument was the over abuse of factory methods.

The method you introduce looks much cleaner and easier to understand. And this is exactly my point. It's OK to use help methods to remove tedious repetions, but when it comes to methods that you must dig in, like those I showed, it is over abusing helper methods.

Steve Ck replied on Tue, 2009/11/24 - 3:27pm

Eyal, instead of factories with multiple signatures for the different options, would the Builder pattern be useful here?


TextField patternTextField = new TextField("pattern");
patternTextField.setRequired(true);
patternTextField.setLabel(new ResourceModel("patternKey"));
 would be replaced with  
TextField patternTextField = new TextFieldBuilder("pattern").
required(true).
label(new ResourceModel("patternKey")).
create();

 If different situations require different sets of required fields, you can subclass the builder,e.g. ErrorFieldBuilder, with a constructor that takes the new requirements. It should be clear in your code that some fields are plain text and others error, password, etc. from the Builder.This can help avoid the particularly bad situation of a sequence of arguments of the same type in the arglist, like the required and isVisible arguments where there is nothing to distinguish incorrect arguments. 

The composite objects could them be constructed from the build components instead of a long list of individual part.  You could get crazy with it and have builder factories that customize the behavior of a builder - a factory builder, for example, that gives the client a customized builder based on some feature  of the browser or user preferences.

 What it might look like:

FormBuilder fbuild = FormBuilderFactory.get(container, localizationId);
FormComponent form = fbuild.add(wicketId, new ErrorFieldBuilder()...build(), behavior, validator).
add(...).
add(...).
create();

 All components can be added with "add" since each can have a different signature based either on the builder or the instantiated type.

Eyal Golan replied on Wed, 2009/12/02 - 6:38am in response to: Steve Ck

The Builder pattern looks interesring.

I will check it out next time I will write a Wicket's Page / Panel.

I want to add that instead of the separate  lines I demonstrated in the first example, almost all methods of Wicket's Component return the component itself. So you can write something like:

 

Component patternTextField = new TextField("pattern").setRequired(true).setLabel(new ResourceModel("patternKey"));

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.