15

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:

  1. 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
        (...)
      }
    }
    
  2. 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();
    
  3. 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:

  1. Is the described problem really a problem?
  2. What do you think about suggested solutions? Which one would you prefer (based on your experience)?
  3. Can you think of any other solution?
  • 1
    Which language are you targeting or is it just sth generell? – Knerd Apr 7 '14 at 11:13
  • No particular language, just general. Feel free to show how features in other languages can help with this. – Ytus Apr 7 '14 at 12:05
  • like I said here programmers.stackexchange.com/questions/235096/… C# and C++ have some features. – Knerd Apr 7 '14 at 12:14
  • Its pretty clear that this question applies to every language which support such kind of method overloading. – Doc Brown Apr 7 '14 at 13:48
  • 1
    @DocBrown ok, but not every language supports the same alternatives ;) – Knerd Apr 7 '14 at 14:01
20

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 :)

  • Thank you. Your answer can be solution num. 4, yes. I'm looking more for answers in this fashion: 'In my experience this leads to ... and can be fixed ... .' or 'When I see this in the code of my colleague, I suggest him to ... instead.' – Ytus Apr 7 '14 at 9:42
  • I don't know... seems to me that you are just moving the problem. Per example, if I can build a document on different parts of the application, it's better to organize and test this isolating this document construction on a separated class (like use a DocumentoFactory). Having a builder on different places it's hard to control the future changes on the Document construction (like add a new mandatory field to document, per example) and add extra boilerplate code on tests to just satisfy the document builder necessities on classes that are using builder. – Dherik Mar 8 at 12:27
1

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.

0

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.

  • 4
    Its a solution, but I don't think its a good solution (at least, not in a context where typesafety is expected). – Doc Brown Apr 7 '14 at 13:44
  • That's right. Because of cases like that, I like overloaded methods or optional parameters. – Knerd Apr 7 '14 at 14:00
  • 2
    Using a dictionary as a parameter is an easy way to reduce the number of apparent parameters to a method, but it obscures the actual dependencies of the method. This forces the caller to look elsewhere for what exactly needs to be in the dictionary, whether that be in comments, other calls to the method, or the method implementation itself. – Mike Partridge Apr 8 '14 at 15:10
  • Use the dictionary only for truly optional parameters so basic use cases need not read too much documentation. – user949300 Jun 18 '18 at 5:38
0

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.

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.