9

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.

  • 1
    One reason I've done that is for COM compatibility, but that doesn't seem to be your reason here. – Mike Jan 7 '16 at 0: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 7 '16 at 1: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 7 '16 at 1: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 7 '16 at 2:08
  • 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: martinfowler.com/bliki/AnemicDomainModel.html – Erik Eidt Jan 7 '16 at 5:32
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.
  • 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 7 '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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.