Question

I'm starting to use AutoMapper and some doubts arose. Where is the correct way to map a dto into a domain model? I'm doing this:

DTO:

public class PersonInsert
{
    [Required]
    public string Name { get; set; }
    public string LastName { get; set; }
}

Action:

[HttpPost]
public ActionResult Insert(PersonInsert personInsert)
{
    if (ModelState.IsValid)
    {
        new PersonService().Insert(personInsert);

        return RedirectToAction("Insert");
    }

    return View("Insert");
}

Service:

public class PersonService
{
    public int Insert(PersonInsert personInsert)
    {
        var person = Mapper.Map<PersonInsert, Person>(personInsert);

        return new PersonRepository().Insert(person);
    }
}

Repository:

 public class PersonRepository
    {
        internal int Insert(Person person)
        {
            _db.Person.Add(person);
            _db.SaveChanges();

            return person.Id;
        }
     }

So, is this correct? should my service knows about domain? or should I make the bind in repository only? is correct to use [Required] in DTO?

Was it helpful?

Solution

Is this correct?

I personally don't see anything wrong with having your service do the mapping

Is it correct to use [Required] in DTO

No, DTOs should have no business logic whatsoever. They should be used purely for transmitting data across different tiers/layers of your application.

DataAnnotations are typically used on ViewModels for client/server side validation, therefore, I would add another separation into your model and introduce a ViewModel for your Insert action e.g.

public class PersonViewModel    
{
    [Required]
    public string Name { get; set; }
    public string LastName { get; set; }
}

public class PersonDto
{
    public string Name { get; set; }
    public string LastName { get; set; }
}

Action:

[HttpPost]
public ActionResult Insert(PersonViewModel personViewModel)
{
     if (ModelState.IsValid)
     {
         var personDto = Mapper.Map<PersonViewModel, PersonDto>(personViewModel);
         new PersonService().Insert(personDto);
         ...
      }
      ...
     }
}

Service:

public class PersonService
{
    public int Insert(PersonDto personDto)
    {
        var person = Mapper.Map<PersonDto, Person>(personDto);

        return new PersonRepository().Insert(person);
    }
}

It may seem overkill in this scenario (considering the only difference is the [Required] attribute). However, in a typical MVC application you would want to ensure a clean separation between your ViewModels and your business models.

OTHER TIPS

I would almost never create an entity from a DTO - I explain why below. I would use a request object to allow a factory method to build the entity:

Request:

public class InsertPersonRequest
{ 
    [Required] 
    public string Name { get; set; } 
    public string LastName { get; set; } 
} 

Action:

[HttpPost] 
public ActionResult Insert(InsertPersonViewModel viewModel) 
{ 
    if (ModelState.IsValid) 
    { 
        InsertPersonRequest request = InsertPersonViewModelMapper.CreateRequestFrom(viewModel);
        new PersonService().Insert(request ); 
        return RedirectToAction("Insert"); 
    } 

    return View("Insert"); 
} 

Service:

public class PersonService 
{ 
    public int Insert(InsertPersonRequest request) 
    { 
        var person = Person.Create(request.name, request.LastName);          
        return new PersonRepository().Insert(person); 
    } 
} 

Repository stays the same.

This way all logic for creating the Person are located in the Factory method of the person, and so business logic is encapsulated in the domain - derived fields, default fields etc.

The problem with what you are doing is that the DTO has to be created in the UI, then all fields are mapped to the entity - this is a sure fire way for business logic to seep into the service layer, UI, or anywhere it is not supposed to be.

PLease read that again - This is a very serious mistake I see made time and time again.

I would however, use AutoMapper in the service layer to return a DTO:

Service:

public class PersonService 
{ 
    public PersonDto GetById(intid) 
    { 
        var person = new PersonRepository().GetById(id);
        var personDto = Mapper.Map<Person, PersonDto>(person); 
        return personDto
    } 
} 

I would say that your PersonService could be seen as part of the domain layer (or Application layer directly above the domain) of your architecture and the controller and DTO is in a layer above that. That means you shouldn't have a reference to the DTO in your PersonService signatures and instead use the domain Person class here. So the Mapping code should go into the Controller. This ensures that your domain logic is not affected by changes to the webservice contract which could really be just one way to use your PersonService. I would also introduce an interface for your repository which is injected into your PersonService because the PersonService again shouldn't need to know about concrete data access implementations.

As for the [Required] attribute, I don't see a problem with having this on the DTO because it just states the data contract of your webservice method. Anybody calling your webservice should adhere to this data contract. Of course this requirement will typically also be reflected somewhere in your domain code, maybe by throwing an exception etc.

In ASP.NET MVC the typical use of DTO is being part of something called viewmodel. Viewmodel is a class that will combine one to several DTOs into one class tailored for view presentation and posting values back to server.

What you doing is correct, no issues with that, but data annotations should reside on view models, rather than DTOs. Unless you call your DTO a view model, then its fine.

Please read the following posting about model (Domain Model) vs ViewModel in ASP.NET MVC world:

Hope this helps

I think it is fine to have annotations on the DTOs, such as [Required], MaxLength, Range etc.

Your DTO can come in from any (possibly untrusted) source (Not just your website, but from another endpoint, WCF service, etc). All requests will be funneled to your Service/Business Layers, so you will need to validate the input before performing your business logic (simple guard checks). Having the annotations on the DTO simply describe the needed input to perform the task at hand. Passing an object with annotations is not peforming validation.

However, I believe you should be validating the DTO information is correct in the service/business layer (and annotations are a nice way to check this).

Just my thoughts on the situation :)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top