3

I'm using MVC 4 with the repository pattern and unit testing also. I have a typical controller that has simple CRUD functionality. I've separated my View Models from my DTOs and I would like to know the best way to convert between the 2:

Models:

I have Admin.Models.Product which is my view model and AdminAssembly.Models.Product which is my DTO.

Controller:

    //repo that handles product operations
    AdminAssembly.Interfaces.IEntityRepository<AdminAssembly.Models.Product> db;
    //default constructor
    public ProductController() { db = new AdminAssembly.Repositories.EntityRepo<AdminAssembly.Models.Product>(new AdminAssembly.Models.EntitiesContext()); }
    //unit testing constructor
    public ProductController(AdminAssembly.Interfaces.IEntityRepository<AdminAssembly.Models.Product> context) { db = context; }

    //
    // POST: /Product/Create

    [HttpPost]
    public ActionResult Create(Admin.Models.Product product) {

        if (ModelState.IsValid) {
            //COMPILE-ERROR: how to convert to DTO?
            db.Add(product);
        }
        return View();

    }

    //
    // GET: /Product/Edit/5

    public ActionResult Edit(int id) {
        //COMPILE-ERROR: how to convert to view model?
        Admin.Models.Product product = db.GetAll().Where(p => p.ID == id);

        return View(product);
    }

How do I convert between the 2?

Do I reference my DTO assembly in my view model and do something like: (won't this break my unit testing?)

//convert to AdminAssembly.Models.Product
db.Add(product.ToDTO());

//convert back to Admin.Models.Product via constructor
Admin.Models.Product product = Admin.Models.new Product(db.GetAll().Where(p => p.ID == id));

Do I need some sort of object conversion black box?

Converter.ToViewProduct(product);

Some sort of interface?

or something else?

Update 1:

    public static class Product {
        public static Admin.Models.Product ToView(AdminAssembly.Models.Product dto) {
            Admin.Models.Product viewProduct = new Admin.Models.Product();
            //straight copy
            viewProduct.Property1 = dto.Property1;
            viewProduct.Property2 = dto.Property2;

            return viewProduct;
        }
        public static AdminAssembly.Models.Product ToDTO(Admin.Models.Product viewModel) {
            AdminAssembly.Models.Product dtoProduct = new AdminAssembly.Models.Product();
            //straight copy
            dtoProduct.Property1 = viewModel.Property1;
            dtoProduct.Property2 = viewModel.Property2;

            //perhaps a bit of wizza-majig
            dtoProduct.Property1 = viewModel.Property1 + viewModel.Property2;

            return dtoProduct;
        }
    }
Phuc Thai
  • 718
  • 7
  • 17
Smithy
  • 2,170
  • 6
  • 29
  • 61

3 Answers3

4

The long-hand response

[HttpPost]
public ActionResult Create(Admin.Models.Product product) 
{
  if (ModelState.IsValid)
  {
    //COMPILE-ERROR: how to convert to DTO?
    var dtoProduct = new AdminAssembly.Models.Product();
    dtoProduct.Property1 = product.Property1;
    dtoProduct.Property2 = product.Property2;
    //...and so on
    db.Add(dtoProduct);
  }
  return View();
}

While this looks verbose and tedious (and it is) it has to happen eventually, somewhere.

You can hide this mapping either in another class or extension method, or you can use a third party like AutoMapper, as Charlino points out.

As a side note, having two classes with the same name in two different namespaces will eventually get confusing (if not for you, then for the next person who has to maintain your code.) Implement friendlier and more descriptive names wherever possible. For example, put all your view models in a folder called ViewModels, not Models. And append all your view models with ViewModel, or VM. It's also a good convention, imo, to name your view models based on the view that they are for, not so much the domain model that they will be mapped to, as not all view models will map directly to a domain model. Sometimes you'll want parts of more than one domain model, for a single view, and that will blow up your naming convention.

So in this particular case I would suggest changing Admin.Models to Admin.ViewModels and then rename the view model version of Product to CreateViewModel. Your code will be much more readable and will not be littered with namespaces throughout your methods.

All of that would result in a method that would look more like this:

[HttpPost]
public ActionResult Create(CreateViewModel viewModel) 
{
  if (ModelState.IsValid)
  {
    var product = new Product();
    product.Property1 = viewModel.Property1;
    product.Property2 = viewModel.Property2;
    //...and so on
    db.Add(product);
  }
  return View();
}
Forty-Two
  • 7,535
  • 2
  • 37
  • 54
  • Thanks, at some point I knew I would have to do this I just wanted to make sure it's testable and *right*. In your opinion would you say Update 1 fits the bill and is also unit testable? – Smithy Mar 08 '13 at 18:25
  • 1
    Also if I prefix all my DTOs with "e" or "dto" would that then match what you propose in your naming conventions? (e.g. Product => dtoProduct) p.s. Many thanks :) – Smithy Mar 08 '13 at 18:27
  • Why do you rename the ViewModels and not the DTOs since I'll have to reference the DTOs far less often? Is it because I still need a convention to deal with separate Create, Edit and Details ViewModels per each DTO? – Smithy Mar 08 '13 at 18:29
  • It's not necessary to change the names of the dto's. If I see a Class called `Product` or `Customer`, I will assume it's a domain object that (probably) maps to the data source. If I see a class called `*ViewModel` I will know it is simply data to be passed to a view. – Forty-Two Mar 08 '13 at 18:30
  • Got ya, I read through your answer in detail and it makes excellent sense. Many thanks for your 42 cents ;) – Smithy Mar 08 '13 at 18:33
  • As far as Edit 1, you're on the right track, but do not make those classes static. That may cause you headaches when unit testing. As a rule of thumb, avoid using static classes unless you actually have a valid reason to do so. You can make your adapter a member variable so you don't have to instantiate a new one every time you call it. – Forty-Two Mar 08 '13 at 18:34
  • Can I make the methods static as instantiating the class seems a bit over the top? – Smithy Mar 08 '13 at 19:55
  • You *can* but that doesn't mean you *should*. Microsoft says "Use a static class as a unit of organization for methods not associated with particular objects." Your methods are most certainly associated with particular objects. Based on that alone I would recommend against it. – Forty-Two Mar 08 '13 at 20:00
3

Check out a library called AutoMapper.

From their wiki:

What is AutoMapper?
AutoMapper is a simple little library built to solve a deceptively complex problem - getting rid of code that mapped one object to another. This type of code is rather dreary and boring to write, so why not invent a tool to do it for us?

Charlino
  • 15,802
  • 3
  • 58
  • 74
  • the getting started guide and wiki links pull back a 404. IF I do take this approach what sort of performance overheads are likely? Can this deal with custom translations I.e. DTOProduct.SpecID = ViewProduct.Date.ToString() + ViewProduct.ID.ToString()? Will it also allow my View Model to only take on a subset of the DTOs Properties if I wish? – Smithy Mar 08 '13 at 17:44
  • I'm pretty sure performance overheads will be negligible. Yes, it should allow for custom transformations, if not you could always use your own get properties on the ViewProduct. And yes, you can allow the ViewModel to only take a subset of the DTO's products. – Charlino Mar 08 '13 at 18:07
  • Hmm interesting, just found out how to reach the wiki pages. Seems to have a lot of useful functionality behind it. My position at the moment is I don't want to learn something just to avoid learning something else if that makes sense. Thanks :) – Smithy Mar 08 '13 at 18:13
1

If you dont want to use AutoMapper you may use extensions, as suggested by @Forty-Two. If the number of things to map is no very great, I would go with this approach, just because then, AutoMapper == YAGNI

public static class Extensions
{
    public static ViewModel ToViewModel(this Model )
    {
        var vm = new ViewModel()
        {
            //map
        };
        return vm;
    }
    public static Model ToModel(this ViewModel viewModel)
    {
        var model = new Model()
        {
            //map
        };
        return model;
    }
}

Similar to your code in UPDATE, but using extensions instead.

Mr Balanikas
  • 1,657
  • 15
  • 28