tags:

views:

182

answers:

3

I've got an object (well 3 similar objects) that inherits from 6 interfaces:

public class PaperContract : 
    IDocument, 
    ICanSell, 
    IPrintable, 
    INotifyPropertyChanged, 
    IDataErrorInfo, 
    ISerializeable

The first signifies that this is a document which has properties for the file format it gets saved to and other stuff that makes it to conform to legacy practices that had to stay consistent with the paper documents this is replacing.

The second signifies that the document has financial info on it.

The third signifies that this document can be printed (seems obvious).

And the rest should be fairly obvious, databinding for winforms and serializing because the app is a sometimes-connected app.

I'm wondering if this is a sign that the object is defying the SRP, but at the same time, I don't want object explosion by splitting the class up into a million smaller sub-classes.

+11  A: 

While I wouldn't say that the absolute number of interfaces would be a code smell, it looks like this object is on its way to becoming a dreaded God Object that contains a lot of information about a contract - how to print it, format it, data bind it, and serialize it. You couldn't use a PaperContractDataBinder, PageContractSerializer, PaperContractFormatter, etc?

The SRP directly relates to small object explosion. Done correctly, you get lots of smaller, more focused classes that do a few very tightly coupled things.

Jarrett Meyer
I agree. This object looks like some of those interfaces could be reconceptualized to be properties.
Paul Nathan
I agree it may not be a problem yet, but maybe you want to create a document object that holds objects for the different tasks like printing etc.
Janusz
Thanks for the input. I took your answer to mean "it's a code smell, but that doesn't mean it's bad - inspect further." When it comes down to it, the class itself only has a (rather large) list of properties, and 3 methods which doesn't seem all that bad. Most of the interfaces just define public properties that the inheriting class must define (credit card number, guid, date, etc). With that in mind I'm going to leave it as is, but keep a watchful eye on it whenever features are added to it.
SnOrfus
+3  A: 

It really depends.

In managed code I would say yes this is definitely a sign of code smell. There is nothing inherently wrong with a class implementing a lot of interfaces but it can be a sign of an object that should split up responsibilities. An example a valid case would be certain collection classes, like Dictionary<TKey,TValue> which implement a lot of interfaces due to the number of scenarios it can validly fill.

In COM programming this unfortunately is how a lot of code is developed. IME, it's common in COM programming for a particular object to be expected to implement a host of COM interfaces in order to achieve full functionality. Especially in applications which maintain APIs accross many releases and add new APIs by adding new interfaces. This forces the author of the object into this model.

JaredPar
+1  A: 

The question to ask is 'is this interface used to provide polymorphism?'

For example; ISerializable is used to allow the serialization system to treat all classes the same (all cast to ISerializable). IDataErrorInfo is used to allow data binding to treat all classes the same to display error messages. INotifyPropertyChanged is used to allow data binding to treat all classes the same to synchronize data.

Is IPrintable used to treat several classes the same way? What about ICanSell?

There is one architectural school of thought that states that classes should have lots of interfaces if they are used by many subsystems\services. This allows a level of decoupling between the services and the classes.

Dr Herbie