How to set default values to all wrong or null parameters of method?

Question

At the moment I have this code (and I don't like it): private RenderedImage

private RenderedImage getChartImage (GanttChartModel model, String title,
                                     Integer width, Integer height,
                                     String xAxisLabel, String yAxisLabel,
                                     Boolean showLegend) {
    if (title == null) {
        title = "";
    }
    if (xAxisLabel == null) {
        xAxisLabel = "";
    }
    if (yAxisLabel == null) {
        yAxisLabel = "";
    }
    if (showLegend == null) {
        showLegend = true;
    }
    if (width == null) {
        width = DEFAULT_WIDTH;
    }
    if (height == null) {
        height = DEFAULT_HEIGHT;
    }
    ...
}

How can I improve it?

I have some thoughts about introducing an object which will contain all these parameters as fields and then, maybe, it'll be possible to apply builder pattern. But still don't have clear vision how to implement that and I'm not sure that it's worth to be done. Any other ideas?

Answer 1

Your method's purpose is to construct a complex object. Therefore, the builder pattern seems appropriate to solve this problem. A builder can manage many options for the creation of an object.

Some properties of the image should not have a default value. For example, an image without a title is not very useful, but this depends on the needs of your application.

The use of a builder could look like:

RenderedImage image = RenderedImageBuilder.getNew(model)
                      .title("title").width(100).height(100)
                      .showLegend().build();

A further advantage of builders is that they make it easy to document any defaults for parameters and how they should be used.

Answer 2

Well, I'm thinking about that is there some framework support @NotNull annotation, if a method has this annotation, the framework will check all it's parameters.

@NotNull public void doSomething(Parameter a, Parameter b) { }

Answer 3

The best I can think of off hand is to introduce a Parameter Object (which will also be a builder) called something like ChartOptions to contain all the options for this method.

The object could be built piecemeal:

ChartOptions options = new ChartOptions()
   .setHeight(10)
   .setWidth(100)

getChartImage(model, options);

etc.

If that doesn't work you can at least encapsulate the null check:

private <A> A checkNull(A object, A default)
{
  return object == null ? default : object;
}

Answer 4

So many parameters to a method is definitely a code smell. I would say a Chart object is waiting to be born. Here is a basic outline:

 private RenderImage getChartImage(Chart chart) { 
     //etc.
 }
 private static class Chart {
      private GanttChartModel model;
      private String title = "";
      //etc, initializing each field with its default value.
      private static class Builder {
           private Chart chart;
           public Builder(GanttChartModel model) {
                chart = new Chart();
                chart.model = model;
           }
           public setTitle(String title) {
                if (title != null) {
                    chart.title = title;
                }
           }
      }
  }

Other options include using primitives on the methods instead of objects to indicate that null isn't allowed, although that doesn't necessarily make it better. Another option is a bunch of overloaded methods, but given the types of parameters here, that doesn't really work because I get the idea that you want to make any of the parameters optional rather than having the first ones required and subsequent ones optional.

Answer 5

I would move that logic into the setter methods of the class you're returning an object of.

public class MyRenderedImage implements RenderedImage {

    public MyRenderedImage(String title, ...) {
        // constructor should call setters that do validation/coercion
    }

    public void setTitle(String title) {
        if (title == null) {
            this.title = "";
        }
    }

    ...
}

Another option to consider is to throw an InvalidArgumentException, but it sounds like you already know what you want to do.

Answer 6

You can have a map values initially constructed. You can then do something like this,

private RenderedImage getChartImage(GanttChartModel model, String title,
        Integer width, Integer height, String xAxisLabel,
        String yAxisLabel, Boolean showLegend) {

    title = removeNull(KEY_TITLE,title);
    xAxisLabel = removeNull(KEY_X,xAxisLabel);
    yAxisLabel = removeNull(KEY_Y,yAxisLabel);
    showLegend = removeNull(KEY_LEG,showLegend);
    width = removeNull(KEY_W,width);
    height = removeNull(KEY_H,height);
}

//initialize the defaultMap with the key-value of default pairs
Map<Object,Object> defaultMap;

private Object removeNull(Object keyTitle, Object value) {
    if(value==null){
        return defaultMap.get(keyTitle);
    }
    return value;
}