Solutions for "too many parameters" warning

Question

In some of my methods, there are Too Many Parameters and very hard to maintain and read source code. And sometimes I am worried about the question "are they passing the appropriate values in the appropriate order?"

I am using Checkstyle as my Eclipse plugin and that gives me warning for more than 7 parameters.

I am not sure it may just be a coding standard and I don't care about it. But when passing many parameters through a view, service or dao, I have noticed that it was very hard to read and hard to modify at later times.

So, I am trying to pass these parameters with...

1. A number of Objects or Beans. But this give me another problems because my parameters wouldn't get any guarantee (not sure whether they are present or not).

2. HashMap type parameters. But this may force me to check some validations and try to match keys from method-call sides.

Above two approaches may also lose compile-time errors checking. Are there any suggestions to reduce parameter counts?

Answer 1

Passing a HashMap is a common practice in untyped scripting languages but is a bad practice in Java. It defeats the advantages of strong typing which is part of how you gain productivity in Java. To put it a different way, the Java compiler won't be able to help you spot mistakes during development and you'll be more likely to catch them at runtime instead.

If the parameters you're passing are related conceptually, you could as you mention group them into an appropriate object. For example if you're passing parameters such as, say, firstName, lastName, dateOfBirth and so on, you can instead pass a Person object which has those properties. This uses OO to make your design easier to think about and maintain.

If I understand what you mean about this: "But this give me another troubles because my parameters would not get any guarantee (not sure will be contain or not)", you can enforce the guarantee you need when your Person or etc. object is instantiated. One approach would be to use an immutable Person (etc.) object: no setters, instead pass all params via the constructor. Throw IllegalArgumentException if they're not all correct.

Good luck!

Answer 2

There are some techniques reduce the # of parameters;

1. Use minimized methods (break the method into multiple methods, each which require only a subset of the parameters)
2. Use utility classes (helper classes) to hold group of parameters (typically static member classes)
3. adapt the Builder pattern from object construction to method invocation.
4. Try to reduce the data flow between separate packages with better architectural design.

Refer some standard java book;

• Java: How To Program
• Head First Java
• Effective Java

Also try to learn design patterns it'll extremely useful as best coding practices.

• Head First Design Patterns

Answer 3

Let me start with some caveats regarding the presented proposals before I present mine. I take the liberty of skipping the "smelly" solutions - HashMap and beans, because you can clearly see their disadvantages.

Be careful with

1. Blindly using helper classes to hold groups of parameters. They can really shine when the group is cohesive (like in Mark Phillips' answer), but they will cause problems otherwise (essentially acting just like a typed HashMap). I doubt that they'd apply to the problem of passing 7 params from view to DAO layer.

2. Minimized methods are also great, when they make sense (like in List example from Effective Java book). I just rarely see places when they do, so I doubt they'd solve your problem.

3. The Builder pattern is often very clean, however it only solves the problem of one layer - it doesn't tell you how to pass parameters further down. And when you have a parameter from view, that's needed in DAO, the Builder will just bloat your code and you will need to pass parameters anyway.

And before I finally present the solution, let me challenge a common implicit assumption, that all data needs to be passed via stack in form of method parameters. That's only true if you make your processing objects application- or session-scoped. The constraint goes away when you create all your relevant objects at the start of request processing. Then you can use objects' constructors to pass only necessary information to them.

The optimal way: Use objects of request lifecycle

To some degree this resembles Method Object or Command pattern.

In order to apply this solution, you need to change the entry point to your system - usually that's a method in some view-layer object. Make it responsible for two things:

• request object graph creation
• calling the object graph root's execute/run method

The first step is crucial. This is the place, where you construct request-scoped objects from each layer: view, service and DAO. For each of those objects you only pass the required data to their constructors (for example, if parameter "userIP" is only needed in DAO - say for auditing DB access, pass it to DAO request object only). The request objects also need references to their collaborators (like service needing the DAO) - pass those via constructor accordingly.

The second step: when you have your object graph set up, just call the execute/run method on the first of them (usually it's an object from view layer).

/** The example (in Scala) shows how your app's entry point could look like.
 *  The presented method belongs to an app-scoped view-layer object.      
 */
def delete(itemId: Id, userIP: IPAddress) {
  // Note, that only RepositoryHelperReq class is interested in the 
  // "itemId" and "userIP" parameters
  val repoReq = MainRepositoryReq(RepositoryHelperReq(itemId, userIP))
  val serviceReq = MainServiceReq(ServiceHelperReq(repoReq))
  val viewReq = MainViewReq(ViewHelperReq(serviceReq))

  viewReq.execute()
}

Now let me respond to some anticipated criticism of this pattern.

Criticism rebuttal

1. Some will say, that performance will suffer, because there will be more object on the heap to garbage collect. I would ask those for measurements, because usually it's not object allocation, that costs performance, but object retention (see last presentation of Simon Ritter).

2. Some will ask about application- or session-scoped data, like datasources or shopping basket object. Those objects still can be used - you just inject those into your request-scoped objects.

3. Some will criticise the dependency structure, saying that view should depend only on service and not on DAO. That's valid remark, only note, that in classic webapps you still have a central place, that depends on every layer used (place often called "end of the world"). Sometimes it's web.xml, other times it's Spring application context or Guice module. If you care for proper dependencies, I advise you to put all the factory logic in such place, to let it implement some View-layer interface and to be injected to the view. That way your whole dependency structure will stay clean and modular.

4. Some will say, that popular DI frameworks (mainly Spring) support that pattern very poorly. That's true, you'll need to either use a decent DI library (Guice, Dagger for Java or Macwire if you fancy Scala) or prepare to fight Spring in order to do it right.

The benefits

1. No long-parameter-list smell
2. No incohesive "request context" objects (the MagicContainer antipattern)
3. No "stamp"-coupling - intermediate layers need not to depend on passed parameters, so they can be released independently, are more testable and cleaner
4. Data is used only where it's needed - easier testing, less mocking
5. Can be used even where other methods fail, like when you do not have cohesive ParameterObject to extract or when you cannot easily split methods into minimal, orthogonal ones

Credits

The solution to this particular problem was introduced to me by Miško Hevery in his excellent blog post How to do Everything Wrong with Servlets and precisely nailed down in Managing Object Lifetimes (see "More common violation" fragment). I want to thank him for his posts, because it's hard to find precise guidance about this specific problem in other sources.

Answer 4

First approach is the way to go, it is essentially encapsulation (one of the basic principals of OO). As far as "But this give me another troubles because my parameters wouldn't get any guarantee (not sure will be contain or not)." - that is a very common problem and has been addressed using JSR 303. Here is a very basic example of a bean with JSR 303 validation annotations:

import javax.validation.constraints.Min;
import javax.validation.constraints.NotNull;

public class Book {

   @NotNull 
   private String title;

   @NotNull
   private String author;

   @Min(value=100)
   private int numOfPages;

   @NotNull
   private String isbn;

   ... ... ...
}

Here is how to validate it:

Book book = new Book();
ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
Validator validator = factory.getValidator();
Set<ConstraintViolation<Book>> violations = validator.validate(book);
for (ConstraintViolation<Book> violation : violations) {
   System.out.format("%s: %s%n",violation.getPropertyPath(), violation.getMessage());
}

And here is the output of running validation:

isbn: may not be null
numOfPages: must be greater than or equal to 100
author: may not be null
title: may not be null

Further reading: http://www.jroller.com/eyallupu/entry/jsr_303_beans_validation_using

You don't have to do such manual validation however, for example in Spring MVC you can put @javax.validation.Valid on a bean being passed into a method and it will get validated automatically.