10

There is a portion of our codebase written in the following style:

// IScheduledTask.cs
public interface IScheduledTask
{
    string TaskName { get; set; }
    int TaskPriority { get; set; }
    List<IScheduledTask> Subtasks { get; set; }
    // ... several more properties in this vein
}

// ScheduledTaskImpl.cs
public class ScheduledTaskImpl : IScheduledTask
{
    public string TaskName { get; set; }
    public int TaskPriority { get; set; }
    public List<IScheduledTask> Subtasks { get; set; }
    // ... several more properties in this vein, 
    // perhaps a constructor or two for convenience.
}

That is, there are a large number of interfaces specifying just a set of properties with no behavior each with a sole corresponding implementation that implements these with auto-properties. The code is written by someone fairly senior (much more so than myself) and is apart from this use of interfaces reasonable procedural code. I was wondering if anyone else had encountered/used this style and if it has any advantages over just using concrete DTOs everywhere without the interfaces.

walpen
  • 3,231
  • 1
  • 11
  • 20
  • 1
    One reason I've done that is for COM compatibility, but that doesn't seem to be your reason here. – Mike Jan 07 '16 at 00:59
  • @Mike Interesting, I've never used COM and it is not used here. I'll ask the author of this section of the code whether he was planning to use the COM or anything. – walpen Jan 07 '16 at 01:05
  • My guess would be that he expects to make at least one of those properties non-auto in the relatively near future, and writing out this boilerplate at the start is a habit he's gotten into to save some time fiddling around later, though I'm not a C# guy so that's all I can say. – Ixrec Jan 07 '16 at 01:09
  • 6
    IMHO it's the over-obsession and misapplication of *code to interfaces not implementation*. A key tell-tale is *the sole implementation*. The point of interfaces is polymorphism but a properties-only class is polymorphic in a sense simply by having different values. Even with behavior - methods - there is no point to it given a single implementation. This nonsense is all over our code base and at best it hinders maintenance. I waste way too much time asking why, what, and where. Why did they do it, what design am I not seeing, and where are the other implementations? – radarbob Jan 07 '16 at 02:08
  • @radarbob makes a good point. Interfaces are for abstraction, so I often follow a rule of: The first class of a given concept, I make implementation only with no separate interface. If a second class of this concept gets written, I'll make an interface to describe their common features. If no second class ever gets written, the implementation-only class is fine by itself. Special cases like COM may force me to make more interfaces than this, but this is the basic scheme. – Mike Jan 07 '16 at 03:04
  • 2
    Most of the preceeding comments go to the single implementation "code smell" of premature abstraction; however, there is also an anemic domain model "code smell" here, see: http://www.martinfowler.com/bliki/AnemicDomainModel.html – Erik Eidt Jan 07 '16 at 05:32
  • To walpen: why don't you ask the senior who has written the code? @radarbob: your comment sounds like a base for a good answer. – Doc Brown Jan 07 '16 at 07:15
  • @walpen What's COM? – Tulains Córdova Jan 07 '16 at 14:38
  • @Mike If you delete the second class because you no longer need it, what do you do? Delete the interface so it no longer is a sole-implementation class? – Tulains Córdova Jan 07 '16 at 14:46
  • @radarbob Polyporphism is not the sole point of interfaces. Decoupling is also a strong point. – Tulains Córdova Jan 07 '16 at 14:50
  • @user61852, The [Component Object Model](https://en.wikipedia.org/wiki/Component_Object_Model) (COM) is a language independent object model designed to enable inter-process/inter-lingual communication. It's kind of a pain to work with, but there are situations in which it is a reasonable choice. – walpen Jan 08 '16 at 14:52

2 Answers2

5

Here's my two cents:

  • We are not sure a DTO will never ever have at least a little bit of behavior. So for me there are objects which may or may not have behavior in the future.
  • One possible cause of having so many sole-implementation classes is a lack of design, for example failing to see that ScheduledTask, ScheduledJob, ScheduledEvent, ScheduledInspection, etc, should be only one segregated Schedulable interface making any implementor schedulable.
  • Creating the interfaces first is a very good practice, it gives you insight of what you need. Many interfaces begin by having no implementation at all. Then someone writes one implementation and they have that one. The state of having a sole implementation is temporary if you avoided what I mention in the second point. How do you know beforehand that some interface will never ever have a second of third implementation?
  • The name we choose for a well thought interface has a tendency to not change in the future, so dependencies to that interface will not be affected. On the other hand a concrete class is prone to be renamed when something important in the way it is implemented changes, for example a concrete class named TaxSheet could change to SessionAwareTaxSheet because a significant overhaul was made but the interface ITaxSheet will probably not be renamed so easily.

Bottom-line:

  • Good design practices apply also to DTOs.
  • DTOs can be added a little behavior down the road.
  • Every interface begins by having only one implementation.
  • In the other hand if you have too many one-interface-one-class combos, there could be a lack of design that should be pointed out. Your preocupation may be justified.
Tulains Córdova
  • 39,201
  • 12
  • 97
  • 154
  • 4
    I upvoted your answer, but your second bullet implies that all classes should automatically have a corresponding interface, a position that I've never agreed with. Writing interfaces on the off-chance that you might have a second implementation merely causes interface proliferation, and YAGNI. – Robert Harvey Jan 07 '16 at 16:15
1

A particular problem I've seen with DTOs that use interfaces is that it allows this:

public interface ISomeDTO { string SomeProperty { get; set; }}

public class SomeDTO : ISomeDTO
{
    public string SomeProperty { get; set; }
    string ISomeDTO.SomeProperty { get { /* different behavior */ } set { SomeProperty = value; } }
}

I've seen this pattern applied as a quick, dirty hack to implement some critical behavior. This leads to code which can have very confusing behavior:

SomeDTO a = GetSomeDTO();
ISomeDTO ia = a;
Assert.IsTrue(a.SomeProperty == ia.SomeProperty); // assert fails!

This is difficult to maintain and confusing to try to untangle or refactor. IMO, adding behavior to DTOs violates the single responsibility principle. The purpose of a DTO is to represent data in a format that can be persisted and populated by a persistence framework.

DTOs are not domain models. We shouldn't be concerned if DTOs are anemic. To quote Martin Fowler's discussion of the anemic domain model:

It's also worth emphasizing that putting behavior into the domain objects should not contradict the solid approach of using layering to separate domain logic from such things as persistence and presentation responsibilities.

Erik
  • 1,405
  • 10
  • 12