Images in this post missing? We recently lost them in a site migration. We're working to restore these as you read this. Should you need an image in an emergency, please contact us at imagehelp@codebetter.com
Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell

A recent paradigm shift I have been trying to make is to using messaging patterns in my project. One impact of this shift has been a more granular approach to various commands against the domain which somehow changes its state. While I do my best to stick to DDD (Domain Driven Design) modeling patterns and concepts I confess I slide into a CRUD based approach to various actions in the system - lots of data in, lots of data out. This is partly due to the nature of the requirements and partly due to lack of creative thinking regarding what business rules should be enforced on the domain.

As I started to examine where my weak points were in my application I realized I was doing an awful lot of mapping between view objects and domain entities within the Application Service layer which was being called by my Controllers in the web app. This is fine for simple cases but collections of value objects or cross-aggregate references can soon result in too much conditional or procedural code. Then I came across this post by Udi Dahan about getting away from CRUD mentality into a more DDD approach. I had read this before but I didn't "get" it so much. Particularly revealing to me was in the comments where Udi explains a method which is returning a interface that feels 'fluent'.

When I looked over the stinkier parts of my application service code, I recognized I could be more creative in the API for creating the variant objects within my aggregate roots while simultaneously enforcing object creation rules in the domain where they belong. An example would serve to illustrate better.

Consider how I might set the "SomeOptionalValueObject" property on the aggregate root below. Also, let's introduce a Specification that the implementation of the value object which may be assigned is subject to the 'SomeState' of the aggregate (the aptly named "SomeStateHasToBeSatisfiedSpecification").

class AggRoot 
{ 
   
public ValueObjectBase SomeOptionalValueObject {get; } 
   
public SomeState SomeState{get;} 
   
public void SetSomeOptionalValueObject(SomeOptionalValueObject obj){
      
//some stuff
       SomeValueObject = obj;
   
}
} 

One of the 'rules' of a Value Object in Domain Driven Design is that only transient references to variant objects may be held. That is, we should treat our aggregate root as the entry point and surface to work on for its state and values contributing that state (ie, 'SomeOptionalValueObject' or 'SomeState' above). This simple discipline acts as a funnel for the rules and integrity for the determined roots in our domain and prevents insanity. :)
 
So how do we perform this simple act of assigning SomeOptionalValueObject to the root which satisfies our SomeStateHasToBeSatisfiedSpecification?
 
First we might simply put a factory method on the root to create the value object determined by its state and then pass that into our Setter method above:
class AggRoot
{
   
//snip
    public ValueObjectBase CreateSomeValueObject(int count,string name, long expectation,TimeSpan duration,/*loads more params*/)
   
{ 
       
//some creation logic
        return valueObj;
   
}
}
Or perhaps create the value object from the arguments and set it in the same method.
 
This works if the value object have a variety of implementations, but gets out of hand if we have even a handful of value objects that require different parameters and types.
public class ValueObjectImpl1 : ValueObjectBase 
{
  
public class ValueObjectImpl1(int count,string name, double amount){}
}
public class ValueObjectImpl2 : ValueObjectBase 
{
  
public class ValueObjectImpl2(TimeSpan duration,long expectation){}
}
public class ValueObjectImpl3 : ValueObjectBase 
{
  
public class ValueObjectImpl3(bool isCoarse,TimeSpan duration, double amount){}
}  

 

If there is only one of these value objects I need to build and add to my aggregate root I might create an interface for a builder that is handed to the client to set the value. The implementation of the builder can enforace the SomeStateHasToBeSatisfiedSpecification for our aggregate root. Additionally this will enforce the signatures of the different value object implementations:

interface IValueObjectBuilder
{
   
void AddImpl1(int count,string name,double amount);
   
void AddImpl2(TimeSpan duration,long expectation);
   
void AddImpl3(bool isCoarse,TimeSpan duration,double amount);
}
class AggRoot
{
   
IValueObjectBuilder ForOptionalValueObject(){
       
//some conditional logic for determining which builder to return
        return builderImplementation;
   
}
}

 

This simple fluency has replaced any property mapping requirement from a client dto => value object and might look now like this:

public void AddSomeValue(object id,ValueObjectImpl1DTO dto)
{ 
   
var aggRoot = repository.Get(id);
   
aggRoot.ForOptionalValueObject()
          
.AddImpl1(2,"mike",40.32);
   
repository.Update(aggRoot);
}
 
I think this is intention-revealing but I could see also that the method chain might be confusing while browsing the AggRoot. To improve this I might change this to:
class AggRoot
{
   
public void SetValueObject(Action<IValueObjectBuilder> with){
     
var builder = GetBuilderBasedOnState();
     
with(builder);
     
SomeOptionalValueObject = builder.Build();
   
}
}

 

Now I only have one method to deal with an I get the bonus of removing the need for the client to call the 'Build' method without resorting to some explicit operator voodoo. Plus I can pass this builder around to do other work internally before pulling the trigger on the build creation method.

This might seem like alot of work but where this starts to shine (I think) is when collections of value objects which all might require different signatures are expected to be added to an aggregate root in one-shot since they are immutable and the collection itself is immutable.

To do this I simply have to create an ISomeValueObjectCollectionBuilder and change the signature on my Set method:

interface ISomeValueObjectCollectionBuilder
{
  
void Add(Action<IValueObjectBuilder> with);
}
class AggRoot
{
  
//snip
   public void SetValueObject(Action<ISomeValueObjectCollectionBuilder> with)
  
{
    
var builder = new SomeValueoBjectCollectionBuilder(this);
    
with(builder);
    
foreach(var item in builder.Items) 
    
{
        
this.items.Add(item);
    
}
  
}
} 
 
Now my client can navigate this API:
 
public void AddSomeValueObjects(Impl1ValueObjectDTO[] dtos)
{
  
//get root
   
  
aggRoot.SetValueObjects(collection=>
   
{
       
foreach(var dto in dtos) {
           
collection.Add(item=>item.AddImpl1(/*params from dto*/);
           
collection.Add(item=>item.AddImpl1(/*params from dto*/);
           
collection.Add(item=>item.AddImpl1(/*params from dto*/);
           
collection.Add(item=>item.AddImpl1(/*params from dto*/);
        
}
   
}
}
   

This simple fluency, along with smarter root factory methods, completely eliminated my need for mappers from DTO objects to their domain variants. Also I found it much simpler to apply specifications on the incoming data AND made creation of these value objects easier to test since it wasn't based on some conditional logic in the service layer. Furthermore, it seems to succeed in meeting the goal of limiting the clients to holding references to the Aggregate Root.

This is of course just one way of enforcing integrity in the domain while translating data from some client in our Application Services using an API that is understandable. A Visitor pattern might be employed to map data based on the IBuilder interface returned by the aggregate root as well without violating separation...but that is another day.

If there are objections to this or perhaps other approaches I'd love to hear them in the comments.


Posted 03-04-2009 11:15 PM by Michael Nichols

[Advertisement]

Comments

Jak Charlton wrote re: Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell
on 03-05-2009 2:37 AM

Mike

I'm sure the intention is good, but it is a little unclear to me what you are trying to achieve, or what you are gaining.

As far as I can see, you are trying to replace a lot of calls like:

order.AddOrderLine(orderLineDTO)

or

order.AddOrderLine(orederLineVO)

I wouldn't put the builder interface on the Aggregate by choice, now it is definitely breaking SRP, even if you are trying to hide it with ISP. Your Entity is now responsible for construction of child objects, and all the things it is actually responsible for.

Why not have:

builder = container.GetBuilderFor<IOrderLine>();

orderLine = builder.Build(2,"mike",40.32);

aggregate.AddNewOrderLine(orderLine);

Or am I missing the point here?

Mike wrote re: Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell
on 03-05-2009 3:20 AM

@Casey-

Thanks for the feedback!

I definitely could build the variant outside the aggregate and pass the value object in (my first take does something similar).

One motivation I had was to prevent creating invalid invariants based on the state of the Aggregate. Your implementation would require validating the value object upon setting it on the aggregate root. I do this for simpler requirements but for more involved conditional logic and construction of value objects I prefer to deal with them at the point of creation; ie, a builder implementation. Also, the introduction of many various implementations of OrderLines in your example introduces conditional complexity and either results in too much conditional mapping code or requires a more sophisticated interface for object construction.

Your point about SRP is good, but I think I disagree since the construction of the object isn't taking place in the aggregate root but just the determination for which object can do the construction. Evans considers putting factory methods on aggregate roots an implementation of his Factory pattern (I know...it's not the Bible :) ) and I think one of the Responsibilities the aggregate root has is supervising the construction of its variants, not just their validation. Not that I propose ALL behaviour has to happen on the root, but it seems simpler to work with the root instead of injecting the factory/builder and doing a conditional state check to get the builder I need. I can just get the root and let it construct the factories according to spec. Testability is the same either way.

Where I have found I like this pattern the most is in dealing wtih collections of value objects. Before any items are added to the aggregate root they are built according to spec. I could pull this collection builder outside the aggregate root, but I think it is simpler for me to understand constructing these collections internally rather than passing them in from the client.

Does that clarify this some or does it still seem too heavy handed?

The project I am on has loads of complex value objects which rely on the state of their aggregate parents for their behavior and construction and this approach has resulted in simpler (thinner) application service methods.

Jak Charlton wrote re: Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell
on 03-05-2009 5:37 AM

Bearing in mind your last paragraph, no it probably isn't too heavy handed ... it does seem like an awful lot of work though and I'm sure with evolution it could be simpler - the idea of Factories from the book obviously is intended for this scenario.

I think I am still confused by the reason for passing the builder to the aggregate for it to use to build the VO/entity ... but then as I said elsewhere - I have a rotten code and probably couldn't understand a simple "if" statement this morning :)

Jak Charlton wrote re: Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell
on 03-05-2009 5:37 AM

rotten COLD ... not rotten code LOL  

DotNetShoutout wrote Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell - Mike Nichols - Son Of Nun Technology
on 03-05-2009 10:58 AM

Thank you for submitting this cool story - Trackback from DotNetShoutout

Victor Kornov wrote re: Is Excessive Mapping Of DTOs/ViewObjects to Entities A Code Smell
on 03-05-2009 11:10 AM

@Mike,

I believe Casey's original point of breaking SRP is still valid. Your Entity (event if it's aggregate root) also takes responsibility of providing the correct builder. I'd think of making this separate explicit service, i.e. BuilderProvider. While your way of passing lambda which passes lambdas to construct a collection is cool ;) I believe separating a builder makes for clearer SoC. Plus, now you can switch/replace builders in the container, which isn't possible (even if it's not needed) in your approach.

Reflective Perspective - Chris Alcock » The Morning Brew #301 wrote Reflective Perspective - Chris Alcock &raquo; The Morning Brew #301
on 03-06-2009 3:25 AM

Pingback from  Reflective Perspective - Chris Alcock  &raquo; The Morning Brew #301

#.think.in wrote #.think.in infoDose #20 (2nd Mar - 6th Mar)
on 03-10-2009 5:44 PM

#.think.in infoDose #20 (2nd Mar - 6th Mar)

About The CodeBetter.Com Blog Network
CodeBetter.Com FAQ

Our Mission

Advertisers should contact Brendan

Subscribe
Google Reader or Homepage

del.icio.us CodeBetter.com Latest Items
Add to My Yahoo!
Subscribe with Bloglines
Subscribe in NewsGator Online
Subscribe with myFeedster
Add to My AOL
Furl CodeBetter.com Latest Items
Subscribe in Rojo

Member Projects
DimeCasts.Net - Derik Whittaker

Friends of Devlicio.us
Red-Gate Tools For SQL and .NET

NDepend

SlickEdit
 
SmartInspect .NET Logging
NGEDIT: ViEmu and Codekana
LiteAccounting.Com
DevExpress
Fixx
NHibernate Profiler
Unfuddle
Balsamiq Mockups
Scrumy
JetBrains - ReSharper
Umbraco
NServiceBus
RavenDb
Web Sequence Diagrams
Ducksboard<-- NEW Friend!

 



Site Copyright © 2007 CodeBetter.Com
Content Copyright Individual Bloggers

 

Community Server (Commercial Edition)