views:

65

answers:

4

Many of my classes end up needing convert functions.

  • DataRow -> Object converters
  • ViewModel <-> Model Converters

My question is where should the functions live?

Option 1: Inside the source class

public class Employee
{
  public EmployeeViewModel ToViewModel() {}
}

var vm = myEmployee.ToViewModel()

Option 2: Inside the destination class

public class EmployeeViewModel
{
  public static EmployeeViewMOdel FromModel() {}
}

var vm = EmployeeViewModel.FromModel(myEmployee);

Option 3: Inside a converter

public class EmployeeModelViewModelConverter
{
  public static EmployeeViewModel ConvertToViewModel(Employee) {}
}
var vm = new EmployeeModelViewModelConverter.ConvertToViewModel(myEmployee);

Option 3 seems to be the cleanest at the cost of having tons of converter classes laying around and either tons of static functions or tons of initialization/IOC injection. It also has the ugliest syntax or you have to add yet another class using extensions.

Clarification: I'm not talking just about the ViewModel/Model classes, but anything where you need to convert one class to another. As another example, I have a rendering system where objects often need to be converted to renderable primitives.

+3  A: 

I believe the Single Responsibility Principle suggests #3, its own converter class.

EDIT: If you need it in a separate method, then I'd hold to what I said above. But @kyoryu has a valid point about the ViewModel, however, I'd agree only if the Model is passed as an argument in the ViewModel constructor, not as a separate method.

John MacIntyre
Possibly... you could also make the argument that the ViewModel itself acts as a Bridge or Adapter, especially since (as I mentioned in my answer) it's likely that the ViewModel will still have a reference to the Employee. If that's not the case, I'd completely agree that SRP suggests option 3.
kyoryu
I concur.......
qstarin
@kyoryu-See my edit. After I read your answer, I started thinking about it, and the method threw me off, this belongs in the ViewModel constructor.
John MacIntyre
+1  A: 

I'd actually probably say option 2. Since the entire point of having a ViewModel is to provide a logical 'translation layer' between the underlying Model and the requirements of the View, it seems to be very much a property of the ViewModel.

Option 1 is (in my opinion) out, as it breaks the separation between the Model and the ViewModel (consider - there are theoretically multiple ViewModels that may need to look at employee data, and they may all be different).

Option 3 is reasonable as well, and certainly gives you even more separation. I'm not entirely sure it's necessary though, as the ViewModel will still likely have an Employee.

kyoryu
+1 for helping me think of the constructors.
John MacIntyre
+1  A: 

You might want to consider something like AutoMapper. Although it was originally developed for mapping between DTO's and ViewModel objects, it is very powerful, and can remove away all that mapping code, without the overhead of so many additional classes.

CubanX
A: 

This might be one of those "personal preference" situations. Each one of your options is implemented within the .NET library, so we can guess there's not a clear-cut best practice from Microsoft's perspective:

  • someCollection.ToArray()
  • DateTime.Parse()
  • Convert.ToInt32()
mikemanne