tags:

views:

271

answers:

5

Suppose I have the following class:

class Camera
{
    public Camera(
        double exposure,
        double brightness,
        double contrast,
        RegionOfInterest regionOfInterest)
    {
        this.exposure = exposure;
        this.brightness = brightness;
        this.contrast = contrast;
        this.regionOfInterest = regionOfInterest;
    }

    public void ConfigureAcquisitionFifo(IAcquisitionFifo acquisitionFifo)
    {
        // do stuff to the acquisition FIFO
    }

    readonly double exposure;
    readonly double brightness;
    readonly double contrast;
    readonly RegionOfInterest regionOfInterest;
}

... and a DTO to transport the camera info across a service boundary (WCF), say, for viewing in a WinForms/WPF/Web app:

using System.Runtime.Serialization;

[DataContract]
public class CameraData
{
    [DataMember]
    public double Exposure { get; set; }

    [DataMember]
    public double Brightness { get; set; }

    [DataMember]
    public double Contrast { get; set; }

    [DataMember]
    public RegionOfInterestData RegionOfInterest { get; set; }
}

Now I can add a method to Camera to expose its data:

class Camera
{
    // blah blah

    public CameraData ToData()
    {
        var regionOfInterestData = regionOfInterest.ToData();

        return new CameraData()
        {
            Exposure = exposure,
            Brightness = brightness,
            Contrast = contrast,
            RegionOfInterest = regionOfInterestData
        };
    }
}

or, I can create a method that requires a special IReporter to be passed in for the Camera to expose its data to. This removes the dependency on the Contracts layer (Camera no longer has to know about CameraData):

class Camera
{
    // beep beep I'm a jeep

    public void ExposeToReporter(IReporter reporter)
    {
        reporter.GetCameraInfo(exposure, brightness, contrast, regionOfInterest);
    }
}

So which should I do? I prefer the second, but it requires the IReporter to have a CameraData field (which gets changed by GetCameraInfo()), which feels weird. Also, if there is any even better solution, please share with me! I'm still an object-oriented newb.

+3  A: 

I usually go about it this way: The business objects are "pure" in the business layer DLL(s). I then add a Camera.MapToCameraDataContract extension method in the WCF layer. I also typcially have the reverse extension method (CameraDataContract.MapToCamera) on the service layer as well.

So in essence, I do it the first way, but the ToData method is an extension method that only the WCF layer knows about.

Daniel Auger
+1. i also use extension methods
Oscar Cabrero
This would require me to expose Camera's fields in some way because extension methods can't access private members of the class they extend, though, which I want to avoid if possible.
Sam Pearson
+8  A: 

I would generally say no, they shouldn't, because DTOs are specific to a service or application, whereas the domain model is your "innermost" layer and should have no dependencies. DTOs are an implementation detail of something other than the domain model, and therefore, it's breaking the abstraction for your domain model to know about them.

Have you considered looking at AutoMapper for this? You'll end up writing a lot less code that way. In this case, I think you'd be able to get away with simply:

Mapper.CreateMap<RegionOfInterest, RegionOfInterestData>();
Mapper.CreateMap<Camera, CameraData>();

And later on:

CameraData cd = Mapper.Map<Camera, CameraData>(camera);

This not only reduces the code churn but compartmentalizes the mapping code in its own "mapping layer" - you have one or several modules that register these mappings that you can put in whichever assembly really uses the DTOs.

And of course you can always create extension methods to simplify the actual mapping:

public static class CameraExtensions
{
    public static CameraData ToCameraData(this Camera camera)
    {
        return Mapper.Map<Camera, CameraData>(camera);
    }
}

Which makes the whole thing as simple as writing camera.ToCameraData(), but without creating a hard dependency between the domain object (Camera) and the DTO (CameraData). You have basically all of the ease-of-use of your original version, but without the coupling.

If you're creating these dependencies because you're trying to create the CameraData object from private Camera data which isn't exposed publicly then my immediate reaction would be, something's not quite right about this design. Why not expose read-only properties on the Camera object? If you're giving the outside world access to them anyway, through the ToData method, then you're clearly not hiding this information, you're just making it more cumbersome to get to.

What if you decide 3 months down the road that you need a different kind of DTO? You shouldn't have to modify your domain-centric Camera object every time you want to support a new use case. Better, in my opinion, to put a few read-only public properties in the class so that mappers can get access to the attributes they need.

Aaronaught
+1 for Automapper. You can also define mapping profiles if you want to separate mappings in a meaningful manner (http://derans.blogspot.com/2009/12/automapper-aspnet-mvc.html) and you can write a unit test to make sure you haven't left some fields unmapped simply by calling **Mapper.AssertConfigurationIsValid();**
R0MANARMY
+1 for combining extension methods with AutoMapper.
Shane Fulmer
This adds needless run time overhead for an abstraction that's never going to make a difference. The DTO is part of the API/contract you establish between parts of the program. If you change the structure of data you're sending between different parts of the program you've changed the design. No amount of automatic mapping is going to prevent you from having to recode if you redesign.
Jay
@Jay: I'm afraid I don't follow your reasoning. DTO = Data Transfer Object, it is a mapping/interchange contract, not part of the domain API. Obviously, if you change your domain object, then you will have to change many other parts of your program; the main reason to use DTOs (constructed from public data on the Domain Object) is to *avoid* having to change your domain model whenever some external dependency changes, and risk regressions or cascading effects. See http://en.wikipedia.org/wiki/Single_responsibility_principle.
Aaronaught
As for adding overhead, yes, it does add some overhead, but if you're running this through a `DataContractSerializer` and a WCF service then you're already talking about several orders of magnitude more overhead than AutoMapper will create.
Aaronaught
I still feel iffy about exposing properties (even readonly ones) that the rest of my domain does not use. By using my second approach, I enforce that the only thing able to read my object's state is an IReporter, rather than (potentially) the entire domain. I realize that somewhere, I'm going to *have* to break object-orientation, because I have to serialize at the service boundary. I'd just prefer to break it in exactly one location, by exactly one type (IReporter). On the other hand, AutoMapper looks pretty freaking sweet.
Sam Pearson
@Sam: There's always more than one way to skin a cat. My take on this is, object data is either public or private. If it's private, don't give it to anybody. If it's public, give it to whoever wants it. Think about what happens if you decide later on that `CameraData` should also contain `ShutterSpeed`; now you have to go and modify your `Camera`, whereas if you'd exposed `ShutterSpeed` as a public read-only property from the start, you would have only had to change the DTO (and mapping).
Aaronaught
I have an idea of where you're coming from with accepting and writing to an `IReporter` interface - however, consider that it doesn't genuinely protect anything about your `Camera`, because anybody can implement the `IReporter` interface. In the end, it's essentially a very roundabout way of accomplishing the same task as providing an accessor property.
Aaronaught
@Aaron, I know what a DTO is. The DTO, which defines the structure of the data returned, is definitely part of the API. How are you going to define what your layer does without defining what it returns?
Jay
@Jay: The domain layer accepts and returns domain objects, not DTOs. Mapping layers use DTOs at the input or output to transform to/from domain objects. The most common example is an ORM - recordsets are a kind of DTO, and the mapper, not the domain model, is responsible for conversion to/from these. It's OK if you don't use this particular architecture and don't care about the distinction between domain and data model, but in such a case it's confusing/misleading to use the terminology at all - it's no longer really a "DTO."
Aaronaught
I broke down and added accessor properties :P I suppose it doesn't matter who uses the accessors since they're read-only. Plus it solves multi-threading issues with writing to a field vs. writing to a local. I just feel like adding them is forcing Camera to implement an implicit "you can make me a DTO" contract. Well I guess at least that's better than forcing Camera to implement an explicit "you can make me a DTO" contract (`ToData()` inside the object).
Sam Pearson
@Aaron, This architecture creates arbitrary distinctions which you then have to create conversions for? That adds inefficiency and gives no value to the users of the program. You're also adding loads of meaning to "DTO". The words "Data Transfer Object" imply nothing about the use or source of the data. You don't seem capable of reducing complexity in anything you do. When did they stop teaching KISS in schools?
Jay
@Jay: If you don't like that architecture then feel free not to use it in your own applications. The definition of a DTO is clear, it's a pattern used to *transfer* data *between* subsystems. If you don't want to listen to me, then read Microsoft's guide, where they describe exactly the same pattern I have, except they refer to the Mapper as an *Assembler*: http://msdn.microsoft.com/en-us/library/ms978717.aspx
Aaronaught
@Jay: With respect to efficiency, it's no more or less efficient than the coupled approach. With respect to complexity, it's far less complex to have a dedicated mapper (again, SRP). With respect to creating arbitrary distinctions, yes, that's our job as programmers. With respect to adding value for users, code quality never does, it simply reduces maintenance costs. And with respect to my personal capabilities, they're hardly relevant to the quality or correctness of this answer, and for those who believe that they are, I'll stand on my record.
Aaronaught
@Sam: Sounds like you're seeing the bigger picture now. :) Generally speaking, any public-facing domain or data object should be convertible to or from an arbitrary DTO, it's an implicit contract of the domain model itself because the domain model is (or should be) designed to be reused in many places.
Aaronaught
A: 

The first (exposing a DTO) is much preferable to me. It's simpler, will run faster, will be easier to understand and maintain. Since the DTO really has no dependency on the database object it still easily achieves the objective of reducing dependencies.

Jay
It doesn't reduce dependencies, it *creates* dependencies - specifically, a dependency of a domain object on a specific DTO.
Aaronaught
The DTO is part of the contract between you and the data layer. Adding Automapper/Hibernate adds a dependency to a whole new code base not just a single object whose structure is fixed by your API.
Jay
The DTO does not necessarily refer to a data layer at all; for example, DTOs are also generally used as View Models in an MVC architecture. "Adding Automapper/Hibernate" (relevance?) does "add a dependency", but I'm not sure I see your point; the dependency is external to the domain object, at a different level entirely. Are you suggesting that we should avoid frameworks and rewrite our code to do everything, or is there something else I'm missing?
Aaronaught
Adding automapper(or any domain mapping code library) adds a dependency to the automapper code library. The programmer has to learn to use automapper and go through the configuration to set it up. The compiler has to chug through more code to produce the application. Add in the run time cost of loading the mapping code every time you start the app, reading the configuration files, and executing the mapping. I'm bewildered how anyone could recommend such a thing.
Jay
A: 

I put the to/from methods on my DTOs themselves:

[DataContract]
public class CameraData
{
    ...
    public Camera ToCamera() { ... }

    public static CameraData FromCamera(Camera c) { ... }
}

That way my domain objects do not have to know about my DTOs.

Gabe Moothart
A: 

Are your services WCF to WCF?

If they are then you could choose to use your business objects as DTOs (as long as your business objects are persistence ignorant). If you did this I'd recommend changing your CameraData class to ICameraData interface and make Camera implement ICameraData. Keep the attributes (DataContract etc) on the interface.

Then you can pass the business object from client to server. Be aware of any logic that is specifically client or server side.

The first image in my blog post here shows how easy it is to Re-Use your business objects objects client side (the dialog box is what is shown when you do 'add service reference'). The blog post has some info on one of the gotchas of re-using biz objects.

I can't tell what you're trying to achieve with ExposeToReporter but you are right, it doesn't look right, personally I'd put a method on IReporter that takes an ICameraData parameter, then set the details on reporter within that.

An awesome source of learning for this stuff is dnrtv. Watch everything with WCF in the title but especially Extreme WCF by Miguel Castro!

wallismark