Interface parameters vs. Construction parameters

I'm going to start with a simple idea for my first post so I can get the hang of things before I jump into tougher topics. My intent is to address issues I have found in real code.

A programmer is working on a new feature and in order to make that feature work he needs to change a currently existing method Work in class Foo. His change makes that method use a reference to another class Bar. There are lots of ways that Foo can get access to the Bar it needs. The two I am going to focus on are "Passing the Reference into the Method" and "Passing the Reference into the Constructor and Storing as a Member".

Passing the Reference into the Method
This approach is easy to implement and the obvious way to get the reference. Unfortunately, it also changes Foo's interface. This is a 'bad thing' if Foo implements an interface (IFoo) that is also implemented by other classes. It also means that the method might be lying about what it needs to do its work.

By adding the Bar reference to its parameter list, Foo is saying that in order for the Work concept to happen anyone who implements it will need a Bar. If this is the case then, of course, Bar should be passed in to Foo. Otherwise, I believe, it should not.

If you add Bar to IFoo::Work (which you will have to do in order to take advantage of polymorphism), all classes that implement IFoo will have to accept that parameters also. More importantly, any class that calls IFoo::Work will have to provide a Bar reference. If the programmer were to repeat this pattern then IFoo::Work would have a reference to every object used by all implementations of Work. Clearly this is far from ideal and significantly increases the complexity of our program. What is the alternative?

Passing the Reference into the Constructor and Storing as a Member
If the programmer passes the reference to Bar into the constructor of Foo and stores it as a member variable then he can use it freely in Work or any other method that happens to need it. The only real downsides of this approach are that you need to have the reference available when you construct each Foo and it also consumes a little bit of extra memory for each instance of Foo.

An added benefit of moving parameters to the constructor is that the parameters of the Work method are more likely to fit into available registers when it is invoked. Since constructors are generally called less often than other methods this benefit is likely to make your program run faster.

The point I am trying to make is that an interface should only have the methods that are intrinsic to the concept that the class embodies. Likewise, the parameters on a method should only be the ones needed to do the work described by the method name.

Comments

[Marc]

It sounds like you are coming from an environment where people are pushing too much into method arguments. I often see too much being pushed into constructor args when they should have been method arguments. But your message is spot on.

Concerning member variables: if the class is hanging on to something, it should probably have full ownership over it, including ownership over its lifetime. Just as one can ask if a method parameter makes sense in the interface, one can ask if it makes sense for the object to own what is being held onto as a member variable.

E.g. say we have a Renderable interfaces and we want to decide whether it take a Renderer in the constructor or as a parameter to the Render method. I would argue that a Renderable does not own the Renderer, so it should never store a strong reference to a Renderer as a member. In this case, I would favor passing a reference to a renderer into the Render method, though I can also see a case for Renderable's hanging onto a weak reference to a Renderer, as it makes sense that a Renderable may own and control the lifetime of a weak reference to the Renderer. But it doesn't make sense that a Renderable owns or controls the lifetime of the Renderer.

When I have something that doesn't seem like it should be owned by the object, nor should it be passed into an interface method of the object, it is usually a sign that a new class with a new responsibility wants to emerge.

[Dan]

@Bill

Right on!

@Marc

> Concerning member variables: if the class is hanging on to something, it should probably have full ownership over it, including ownership over its lifetime.

That seems particularly important in C++, less true in GC or ref-counted environments (such as Objective-C).

What if 2 instances are peers? How can you represent one instance "knowing" about another instance?

[Matthew]

> By adding the Bar reference to its parameter list, Foo is saying that in order for the Work concept to happen anyone who implements it will need a Bar. If this is the case then, of course, Bar should be passed in to Foo. Otherwise, I believe, it should not.

That's one way to look at it. Another interpretation that I sometimes find superior is this: By taking Bar as a Work parameter, Foo is saying that every caller of Work supplies a Bar, even when the Bar isn't going to be used, and that the system is currently arranged so that nothing has to go out of its way to supply the Bar, whether or not it will be used. In other words, I sometimes focus entirely on what is, rather than on what will or should be. This works best when I'm working in an environment where refactoring is easy, on a project that has been constantly tuned via refactoring, on a team full of people who like to refactor very often and who are very good at refactoring--because then I know that as soon as the Bar requirement that "should not" be there actually gets in the way, it will be refactored into a new configuration that works better, with minimal trouble. So the concept of "should" never quite gets a chance to be useful.

Unfortunately, this set of conditions only seems to hold when I'm working by myself. :(

> If the programmer were to repeat this pattern then IFoo::Work would have a reference to every object used by all implementations of Work. Clearly this is far from ideal and significantly increases the complexity of our program.

I do it that way sometimes. It works fine, when it works fine. When it doesn't, yeah, constructor parameters are nice.

> I would argue that a Renderable does not own the Renderer, so it should never store a strong reference to a Renderer as a member.

So don't call it a Renderable--think of something that "should" hold a reference to a Renderer, and name it after that.