Constructing an object graph from a flat DTO using visitor pattern
Asked Answered
H

4

12

I've written myself a nice simple little domain model, with an object graph that looks like this:

-- Customer
    -- Name : Name
    -- Account : CustomerAccount
    -- HomeAddress : PostalAddress
    -- InvoiceAddress : PostalAddress
    -- HomePhoneNumber : TelephoneNumber
    -- WorkPhoneNumber : TelephoneNumber
    -- MobilePhoneNumber : TelephoneNumber
    -- EmailAddress : EmailAddress

This structure is completely at odds with the legacy database I'm having to work with, so I've defined a flat DTO which contains the data for each element in the customer graph - I have views and stored procedures in the database which allow me to interact with the data using this flat structure in both directions, this all works fine & dandy :)

Flattening the domain model into a DTO for insert/update is straightfoward, but what I'm having trouble with is taking a DTO and creating the domain model from it... my first thought was to implement a visitor which would visit each element in the customer graph, and inject values from the DTO as necessary, something a bit like this:

class CustomerVisitor
{
    public CustomerVisitor(CustomerDTO data) {...}

    private CustomerDTO Data;

    public void VisitCustomer(Customer customer)
    {
        customer.SomeValue = this.Data.SomeValue;
    }

    public void VisitName(Name name)
    {
        name.Title     = this.Data.NameTitle;
        name.FirstName = this.Data.NameFirstName;
        name.LastName  = this.Data.NameLastName;
    }

    // ... and so on for HomeAddress, EmailAddress etc...
}

That's the theory and it seems like a sound idea when it's laid out simply like that :)

But for this to work the entire object graph would need to be constructed before the visitor erm, visited, otherwise I'd get NRE's left right and centre.

What I want to be able to do is let the visitor assign objects to the graph as it visits each element, with the goal being to utilize the Special Case pattern for objects where data is missing in the DTO, eg.

public void VisitMobilePhoneNumber(out TelephoneNumber mobileNumber)
{
    if (this.Data.MobileNumberValue != null)
    {
        mobileNumber = new TelephoneNumber
        {
            Value = this.Data.MobileNumberValue,
            // ...
        };
    }
    else
    {
        // Assign the missing number special case...
        mobileNumber = SpecialCases.MissingTelephoneNumber.Instance;
    }
}

Which I honestly thought would work, but the C# throws me an error on:

myVisitor.VisitHomePhone(out customer.HomePhoneNumber);

Since you can't pass ref/out parameters in this way :(

So I'm left with visiting independent elements and reconstructing the graph when its done:

Customer customer;
TelephoneNumber homePhone;
EmailAddress email;
// ...

myVisitor.VisitCustomer(out customer);
myVisitor.VisitHomePhone(out homePhone);
myVisitor.VisitEmail(out email);
// ...

customer.HomePhoneNumber = homePhone;
customer.EmailAddress = email;
// ...

At this point I'm aware that I'm quite far away from the Visitor Pattern and am much closer to a Factory, and I'm starting to wonder whether I approached this thing wrong from the start..

Has anyone else run into a problem like this? How did you overcome it? Are there any design patterns which are well suited to this scenario?

Sorry for posting such a looong question, and well done for reading this far :)

EDIT In response to the helpful answers from Florian Greinacher and gjvdkamp, I settled on a relatively simple factory implementation that looks like this:

class CustomerFactory
{
    private CustomerDTO Data { get; set; }

    public CustomerFactory(CustomerDTO data) { ... }

    public Customer CreateCustomer()
    {
        var customer = new Customer();
        customer.BeginInit();
        customer.SomeFoo = this.Data.SomeFoo;
        customer.SomeBar = this.Data.SomeBar
        // other properties...

        customer.Name = this.CreateName();
        customer.Account = this.CreateAccount();
        // other components...

        customer.EndInit();
        return customer;
    }

    private Name CreateName()
    {
        var name = new Name();
        name.BeginInit();
        name.FirstName = this.Data.NameFirstName;
        name.LastName = this.Data.NameLastName;
        // ...
        name.EndInit();
        return name;
    }

    // Methods for all other components...
}

I then wrote a ModelMediator class to handle interaction between the data layer and the domain model...

class ModelMediator
{
    public Customer SelectCustomer(Int32 key)
    {
        // Use a table gateway to get a customer DTO..
        // Use the CustomerFactory to construct the domain model...
    }

    public void SaveCustomer(Customer c)
    {
        // Use a customer visitor to scan for changes in the domain model...
        // Use a table gateway to persist the data...
    }
}
Hartmann answered 2/6, 2011 at 15:31 Comment(6)
Would something like Automapper work for you instead? automapper.codeplex.comFrizette
@Frizette thanks for the response, I have evaluated automapper (and valueinjecter) but unfortunately neither is suitable, because my domain objects implement ISupportInitialize, and I need to BeginInit() before injecting any of the properties from the DTO, and EndInit() afterwards - I don't think this is possible with Automapper :(Hartmann
If you're using .net 4 then I'd suggest the Lazy<T> class will help you along a bit :)Athanasius
@Hartmann imho CustomerDTO data should be received in the CreateCustomer method, not in the factory constructor.Dreiser
@Dreiser I had considered that. In that case the entire factory could be made static.. would there be any specific advantage to this?Hartmann
@Hartmann personally I'm biased towards DI, so I usually avoid static and stick to regular instances passed by DI to the constructor (in this case of ModelMediator). This said, it is by nature static (no state to maintain), changing it to that means less instances. Only on specific scenarios that's really a difference. It's a whole different question :)Dreiser
T
7

I think you are really over-complicating things here. Just use a factory method and let your domain objects clearly state on which other domain objects they depend.

class Customer
{
    private readonly Name name;
    private readonly PostalAddress homeAddress;

    public Customer(Name name, PostalAddress homeAddress, ...)
    {
        this.name = name;
        this.homeAddress = homeAddress;
        ...
    }
}

class CustomerFactory
{
    Customer Create(CustomerDTO customerDTO)
    {
        return new Customer(new Name(...), new PostalAdress(...));
    }
}

If you need to take a dependency from Customer to CustomerDTO pass the DTO as additional argument to the constructor, probably wrapped in an additional abstraction.

This way things will keep clean, testable and easy to understand.

Tittivate answered 6/6, 2011 at 10:1 Comment(2)
thanks for your answer, you are right I am letting things get too complicated. I do not want the domain model classes to ahve any knowledge of the DTO, so there must be some mediator which can map between them.. I think a factory class as you mention is the way to procede :)Hartmann
I have come to a solution and put my answer in the original question. Because yourself and gjvdkamp both helped me in equal measures, I'm going to leave the bounty to expire, at which point it will automatically go to the answer with the most votes. I think this is the most fair way :)Hartmann
T
5

I don't think i would go with a visitor. That would be appropriate if you don't know at design time, what operations you need to perform on it later, so you open up the class to allow for others to write visitors that implement that logic. Or there are so many things that you need to do on it that you don't want to clutter your class with this.

What you want to do here is create an instance of a class from a DTO. Since the structure of the class and the DTO are closely linked (you do your mapping in the DB, I assume you handle all mapping issues on that side and have a DTO format that maps directly to the structure of your customer), you know at design time what you need to. There's no need for much flexibility. (You want to be robust though, that the code can handle changes to the DTO, like new fields, without throwing exceptions)

Basically you want to construct a Customer from a snippet of a DTO. What format do you have, just XML or something else?

I think I would just go for a constructor that accepts the DTO and returns a Customer (example for XML:)

class Customer {
        public Customer(XmlNode sourceNode) {
            // logic goes here
        }
    }

The Customer class can 'wrap around' an instance of the DTO and 'become one'. This allows you to very naturally project an instance of your DTO into a customer instance:

var c = new Customer(xCustomerNode)

This handles the high level pattern choice. Do you agree so far? Here's a stab at the specific issue you mention with trying to pass properties 'by ref'.I do see how DRY and KISS can be at odds there, but I would try not to overthink it. A pretty straight forward solution could fix that.

So for the PostalAddress, it would have it's own constructor too, just like the Customer itself:

public PostalAddress(XmlNode sourceNode){
   // here it reads the content into a PostalAddress
}

on the customer:

var adr = new PostalAddress(xAddressNode);

The problem I see here is, where do you put the code that figures out if this if the InvoiceAddress or the HomeAddress? This does not belong in the constructor of the PostalAddress, because there could be other uses for the PostalAddress later, you don't want to hardcode it in the PostalAddress class.

So that task should be handled in the Customer class. This is where he usage of the PostalAddress is determined. It needs to be able to tell from the returned Address what type of address it is. I guess the simplest approach would be to just add a property on PostalAddress that tells us:

public class PostalAddress{
  public string AdressUsage{get;set;} // this gets set in the constructor

}

and in the DTO just specify it:

<PostalAddress usage="HomeAddress" city="Amsterdam" street="Dam"/>

Then you can look at it in the Customer class and 'stick it' in the right property:

var adr = new PostalAddress(xAddressNode);
switch(adr.AddressUsage){
 case "HomeAddress": this.HomeAddress = adr; break;
 case "PostalAddress": this.PostalAddress = adr; break;
 default: throw new Exception("Unknown address usage");
}

A simple attribute that tells the Customer what type of address it is would be enough I guess.

How does it sound so far? Code below puts it all together.

class Customer {

        public Customer(XmlNode sourceNode) {

            // loop over attributes to get the simple stuff out
            foreach (XmlAttribute att in sourceNode.Attributes) {
                // assign simpel stuff
            }

            // loop over child nodes and extract info
            foreach (XmlNode childNode in sourceNode.ChildNodes) {
                switch (childNode.Name) {
                    case "PostalAddress": // here we find an address, so handle that
                        var adr = new PostalAddress(childNode);
                        switch (adr.AddressUsage) { // now find out what address we just got and assign appropriately
                            case "HomeAddress": this.HomeAddress = adr; break;
                            case "InvoiceAddress": this.InvoiceAddress = adr; break;
                            default: throw new Exception("Unknown address usage");
                        }    
                        break;
                    // other stuff like phone numbers can be handeled the same way
                    default: break;
                }
            }
        }

        PostalAddress HomeAddress { get; private set; }
        PostalAddress InvoiceAddress { get; private set; }
        Name Name { get; private set; }
    }

    class PostalAddress {
        public PostalAddress(XmlNode sourceNode) {
            foreach (XmlAttribute att in sourceNode.Attributes) {
                switch (att.Name) {
                   case "AddressUsage": this.AddressUsage = att.Value; break;
                   // other properties go here...
            }
        }
    }
        public string AddressUsage { get; set; }

    }

    class Name {
        public string First { get; set; }
        public string Middle { get; set; }
        public string Last { get; set; }
    }

and a snippet of XML. You haven't said anything about your DTO format, would work for other formats too.

<Customer>  
  <PostalAddress addressUsage="HomeAddress" city="Heresville" street="Janestreet" number="5"/>
  <PostalAddress addressUsage="InvoiceAddress" city="Theresville" street="Hankstreet" number="10"/>
</Customer>

Regards,

Gert-Jan

Turnstone answered 6/6, 2011 at 9:1 Comment(4)
Hi thanks for your answer. My DTO as you can see from my question is a POCO class rather than XML, but XML aside your answer is basically the same as the answer from Florian. I would like to try and avoid having an extra constructor in the domain model which accepts a DTO, as I would prefer them to have no knowledge of eachother. I think there should be some mediator between the data and the models which is able to convert in both directions...Hartmann
Hi, then insetad of a contructor on the class itself you could have static method that returns a Customer in a seperate mapping class. Otherwise the logic would stay pretty much the same, although you might run into a few encapsulation issues because you're no longer in the class itself. You could solve that by making them protected instead of private and derive the mapping class from Customer. That's my gripe with the visitor pattern: to actually make it work you often have to tear down the encapsulation of your classes or work around it in some other way.Turnstone
Yes I think this is the way to go, a factory implementation which eats a customer DTO and spits out a fully formed Customer domain model. The encapsulation wont be too much of an issue as they'll live in the same assembly and anything the factory needs access to can be internal :)Hartmann
I have come to a solution and put my answer in the original question. Because yourself and Florian both helped me in equal measures, I'm going to leave the bounty to expire, at which point it will automatically go to the answer with the most votes. I think this is the most fair way :)Hartmann
B
2

For doing conversions between a model class and a DTO, my preference is to do one of four things:

a. use an implicit conversion operator (especially when dealing json-to-dotnet transitions).

public class Car
{
    public Color Color {get; set;}
    public int NumberOfDoors {get; set;}        
}

public class CarJson
{
    public string color {get; set;}
    public string numberOfDoors { get; set; }

    public static implicit operator Car(CarJson json)
    {
        return new Car
            {
                Color = (Color) Enum.Parse(typeof(Color), json.color),
                NumberOfDoors = Convert.ToInt32(json.numberOfDoors)
            };
    }
}

and then usage is

    Car car = Json.Decode<CarJson>(inputString)

or more simply

    var carJson = new CarJson {color = "red", numberOfDoors = "2"};
    Car car = carJson;

voila, instant conversion :)

http://msdn.microsoft.com/en-us/library/z5z9kes2.aspx

b. Use linq projection to change the shape of the data

IQueryable<Car> cars = CarRepository.GetCars();
cars.Select( car => 
    new 
    { 
        numberOfDoors = car.NumberOfDoors.ToString(), 
        color = car.Color.ToString() 
    } );

c. Use some combination of the two

d. Define an extension method (that could also be used in the linq projection)

public static class ConversionExtensions
{
    public static CarJson ToCarJson(this Car car)
    {
        return new CarJson {...};
    }
}

CarRepository.GetCars().Select(car => car.ToCarJson());
Bitter answered 9/6, 2011 at 0:55 Comment(2)
Both good suggestions :) The implicit cast operator is nice but does require the DTO to have intimate knowledge of the domain model, which is a no-no for me. The linq projection idea is actually a really good idea which I'm going to use when dealing with collections of DTO's, although the linq expression will simply defer to the CustomerFactory to make the conversion...Hartmann
I hear you. Personally, I'm ok with a little more coupling since a DTO is so closely related to the model; also, the ability to add multiple two way conversions is nice. p.s., I added another option above that I use from time to time along w/ linq projection. G'luck!Bitter
E
0

You could take the approch I described here: convert a flat database resultset into hierarchical object collection in C#

The idea behind is to read an object, like Customer and put it into a Dictionary. When reading the data for e.g. CustomerAccount, you can now take the Customer from the Dictionary and add the Customer Account to the customer.

You'll have only one iteration over all data to build your object graph.

Electrolyte answered 6/6, 2011 at 8:59 Comment(1)
This is a little different from my scenario - I have a finite set of discreet values representing each data point in the hierarchical graph, rather than a series of similar data points which need to be added to a collection. But thanks anyway!Hartmann

© 2022 - 2024 — McMap. All rights reserved.