mtsz
mtsz

Reputation: 2855

Is it a code smell to have a special constructor only used during testing?

Assume I have a class Foo which is only instantiated with an instance of class Bar:

public Foo(Bar x) {
    this.a = x.a();
    this.b = x.b();
    ...
}

Now I would like to test Foo, further assuming an instance of Bar with the desired state is difficult to create. As an additional constraint, the fields a, b, ... are declared as final, so setters for this fields are not available.

A possibility would be to create an additional constructor in Foo:

protected Foo(A a, B b, ...) {
    this.a = a;
    this.b = a;
    ...
}

This constructor is only used during testing, which I would declare in the comment for this constructor.

Question: Is this a code smell?

Another solution I was thinking of was mocking Bar. Wonder if its the best practice in this case?

Upvotes: 5

Views: 2169

Answers (3)

Erwin Smout
Erwin Smout

Reputation: 18408

Hmmmmmm.

  • If the problem is that Bars are difficult/cumbersome to create, then make it easy to create them.
  • Why don't you have a constructor as well that has the whatever number of arguments that are taken from the Bar object ? It looks like it's a wrapper or Dao kind of thing. You can avoid the 20- or 30- arg invocations by using the wrapper, but how are your invokers going to create the wrapper (without using the 20- or 30-arg invocation for creating the wrapper or using a 20- or 30- level chaining of "return this" setters) ?

That said, what kind of tests are you speaking of ? If the thing-to-be-tested is individual methods or the thing-to-be-tested is the entire application in the most production-like setting we can get, that makes a serious difference as to what it takes to create the environment for the thing-to-be-tested. "As production-like as we can get" means specifically you should not be executing any code whose purpose is just to make the test succeed. That is somewhat less of a problem in tests targeted at smaller individual parts of the application. "Testing" is not always the same thing as "testing". Testing rollbars for cars means stuf like testing whether it can take the torque it says it can take on the tin. You don't do the same stuff when testing the entire car. Testing a part is not the same thing as testing the entire assembly.

Upvotes: 0

Mike M
Mike M

Reputation: 1778

I would probably consider that poor form, but if it works, it works. A preferable solution would be making a TestBar class that is a sub-class of Bar and overrides the methods in Bar. I guess that's pretty much mocking the object, as you alluded to in your question. It's a lot cleaner and allows you to define a different Bar implementation that can be included by your test driver.

Upvotes: 4

Peter Lawrey
Peter Lawrey

Reputation: 533660

Mocking Bar is more likely to be considered best practice. You should be able to create a MockBar so you can do

Foo foo = new Foo(new MockBar(a, b));

Upvotes: 9

Related Questions