Testable Java Code: using model beans with a constructor

Question

According to Misko Hevery that has a testability blog. Developers should avoid 'holder', 'context', and 'kitchen sink' objects (these take all sorts of other objects and are a grab bag of collaborators). Pass in the specific object you need as a parameter, instead of a holder of that object.

In the example blow, is this code smell? Should I pass only the parameters that are needed or a model/bean with the data that I need.

For example, would you do anything like this: Note. I probably could have passed the data as constructor args. Is this a code smell?

public Parser {
  private final SourceCodeBean source;

  public Parser(final SourceCodeBean s) {
    this.source = s;
  }

  public void parse() {
     // Only access the source field
     this.source.getFilename();
     ...
     ... assume that the classes uses fields from this.source
     ...
  }

}

public SourceCodeBean {
   private String filename;
   private String developer;
   private String lines;
   private String format;

   ...
   ...
   <ONLY SETTERS AND GETTERS>
   ...
}

...

Or 

public Parser {


  public Parser(String filename, String developer, String lines ...) {
    ...
  }

}

And building a test case

public void test() {
  SourceCodeBean bean = new SourceCodeBean():
  bean.setFilename();

  new Parser().parse();
}

Another question: With writing testable code, do you tend to write TOO many classes. Is it wrong to have too many classes or one class with too many methods. The classes are useful and have a single purpose. But, I could see where they could be refactored into one larger class...but that class would have multiple purposes.

Answer 1

You will also notice that Misko Hevery advises to group parameters in classes, whenever the parameter count increases or in cases where this is logically acceptable.

So in your case, you can pass the SourceCodeBean without remorse.

Answer 2

A lot of what you are asking is highly subjective, and it is difficult to make useful suggestions without knowing the full scope of what you are trying to accomplish but here is my 2 cents.

I would go with your latter design. Create one class called SourceCodeParser, have the constructor take in filename, developer, etc, and have it have a parse method. That way the object is responsible for parsing itself.

Typically I prefer to pass in parameters to the constructor if they are not too numerous. Code Complete recommends a max of 7 parameters. If you find the number of constructor parameters to be cumbersome you can always create setters off of the fore-mentioned SourceCodeParser class.

If you want a way to institute different parsing behavior I would recommend using a Parser delegate inside of SourceCodeParser and have that be passed in as either a constructor parameter or a setter.

Answer 3

If you have a class who's sole purpose is to associate together various pieces of information, then I see no reason why that class should not be used directly as a parameter. The reason being that the class was coded to do exactly that, so why would you not let it do its job? So I would definitely prefer the former.

Now, this is assuming that the Parser actually needs the information as it's semantically presented in SourceCodeBean. If all the Parser actually needs is a filename, then it should just take the filename, and I would prefer the second method.

I think the only thing that might worry me here is SourceCodeBean becoming a kind of "kitchen sink" of information. For instance, the filename and format fields make perfect sense here. But do you really need the developer and lines? Could those be instead in some sort of associated metadata-information class?