How to avoid excessive method overloading?

We have quite a lot of places in the source code of our application , where one class has many methods with same names and different parameters. Those methods always have all the parameters of a ‘previous’ method plus one more.

It’s a result of long evolution (legacy code) and this thinking (I believe):

“There is a method M that does thing A. I need to do A + B. OK, I know … I will add a new parameter to M, create a new method for that, move code from M to the new method with one more parameter, do the A + B over there and call the new method from M with a default value of the new parameter.“

Here is an example (in Java-like-language):

class DocumentHome {

  (...)

  public Document createDocument(String name) {
    // just calls another method with default value of its parameter
    return createDocument(name, -1);
  }

  public Document createDocument(String name, int minPagesCount) {
    // just calls another method with default value of its parameter
    return createDocument(name, minPagesCount, false);
  }

  public Document createDocument(String name, int minPagesCount, boolean firstPageBlank) {
    // just calls another method with default value of its parameter
    return createDocument(name, minPagesCount, false, "");
  }

  public Document createDocument(String name, int minPagesCount, boolean firstPageBlank, String title) {
    // here the real work gets done
    (...)
  }

  (...)
}

I feel like this is wrong. Not only that we can’t keep adding new parameters like this forever, but the code hard to extend/change because of all the dependencies between methods.

Here are few ways how to do this better:

Introduce a parameter object:

class DocumentCreationParams {

  String name;
  int minPagesCount;
  boolean firstPageBlank;
  String title;

  (...)
}

class DokumentHome {

  public Document createDocument(DocumentCreationParams p) {
    // here the real work gets done
    (...)
  }
}

Set the parameters to the DocumentHome object before we call createDocument()

  @In
  DocumentHome dh = null;

  (...)

  dh.setName(...);
  dh.setMinPagesCount(...);
  dh.setFirstPageBlank(...);

  Document newDocument = dh.createDocument();

Separate the work into different methods and call them as needed:

  @In
  DocumentHome dh = null;

  Document newDocument = dh.createDocument();
  dh.changeName(newDocument, "name");
  dh.addFirstBlankPage(newDocument);
  dh.changeMinPagesCount(new Document, 10);

My questions:

Is the described problem really a problem?
What do you think about suggested solutions? Which one would you prefer (based on your experience)?
Can you think of any other solution?

Maybe try the builder pattern? (note: fairly random Google result 🙂

var document = new DocumentBuilder()
                   .FirstPageBlank()
                   .Name("doc1final(2).doc")
                   .MinimumNumberOfPages(4)
                   .Build();

I cannot give a full rundown of why I prefer builder over the options you give, but you have identified a large problem with a lot of code. If you think you need more than two parameters to a method you probably have your code structured wrongly (and some would argue one!).

The problem with a params object is (unless the object you create is in some way real) you just push the problem up a level, and you end up with a cluster of unrelated parameters forming the ‘object’.

Your other attempts look to me like someone reaching for the builder pattern but not quite getting there 🙂

Using a parameter object is a good way to avoid (excessive) overloading of methods:

it cleans up the code
seperates the data from the functionality
makes the code more maintainable

I would, however, not go too far with it.

Having an overload here and there is not a bad thing. It is supported by the programming language, so use it to your advantage.

I was unaware of the builder pattern, but have used it “by accident” on a few occasions. Same applies here: don’t overdo it.
The code in your example would benefit from it, but spending a lot of time implementing it for every method that has a single overload method is not very efficient.

Just my 2 cents.

I think that the builder solution can work on the most part of scenarios, but on more complex cases your builder will also be complex to setup, because it’s easy to commit some mistakes on the order of the methods, what methods needs to be expose or not, etc. So, many of us will prefer a simplest solution.

If you just create a simple builder to build a document and spread this code on different parts (classes) of the application, it will be hard to:

organize: you will have many classes building a document on different ways
maintain: any change on document instantiation (like add a new mandatory filed) will lead you to a Shotgun Surgery
test: if you are testing a classe that builds a document, you will need add boilerplate code just to satisfy the document instantiation.

But this not answer the OP question.

The alternative to overload

Some alternatives:

Rename the method name: if the same method name is creating some confusion, try to rename the methods to create a meaningful name better than createDocument, like: createLicenseDriveDocument, createDocumentWithOptionalFields, etc. Of course, this can lead you to giant method names, so this is not a solution for all cases.
Use static methods. This approach is kind of similar if compared to the first alternative above. You can use a meaningful names to each case and instantiate the document from a static method on Document, like: Document.createLicenseDriveDocument().
Create a common interface: you can create a single method called createDocument(InterfaceDocument interfaceDocument) and create different implementations for InterfaceDocument. Per example: createDocument(new DocumentMinPagesCount("name")). Of course you don’t need a single implementation for each case, because you can create more than one constructor on each implementation, grouping some fields that makes sense on that implementation. This pattern is called telescoping constructors.

Or Just stay with overload solution. Even being, sometimes, an ugly solution, there is not many drawbacks on using it. In that case, I prefer to use overload methods on a separated class, like a DocumentoFactory that can be injected as dependency on classes that needs to create documents. I can organize and validate the fields without the complexity of create a good builder and maintain the code on a single place too.

I honestly don’t see a big problem with the code. In C# and C++ you can use optional parameters, that would be an alternative, but as far as I know Java doesn’t support that kind of parameters.

In C# you could make all the overloads private and one method with optional parameters is public to call the stuff.

To answer your question part 2, I would take the parameter object or even a dictionary/HashMap.

Like so:

class DokumentHome {

  public Document createDocument(Map<string, object> params) {
    if (params.containsKey("yourkey")) {
       // do that
    }
    // here the real work gets done
    (...)
  }
}

As disclaimer, I am first a C# and JavaScript programmer and then a Java programmer.

I think this is a good candidate for the builder pattern. Builder pattern is useful when you want to create objects of the same type, but with different representations.

In your case, I would have a builder with the following methods:

SetTitle (Document document) { ... }
SetFirstPageBlank (Document document) { ... }
SetMinimumPageCount (Document document) { ... }

You can then use the builder in the following way:

_builder.SetTitle(document);
_builder.SetFirstPageBlank(document);

On the side note, I don’t mind few simple overloads – what the hell, .NET framework uses them all over the place with HTML helpers. However, I would re-evaluate what I’m doing if I have to pass more than two parameters to every method.

Filed under: softwareengineering - @ 07:36

Thẻ: clean-code, legacy-code, refactoring

Thiết kế website giá rẻ

Danh mục

How to avoid excessive method overloading?

The alternative to overload