Should I map a domain object to a view model using an optional constructor?

Question

I'd like to be able to map a domain model to a view model by newing up a view model and passing in the contributing domain model as a parameter (like the code below). My motivation is to keep from re-using mapping code AND to provide a simple way to map (not using automapper yet). A friend says the view model should not know anything about the "payment" domain model that's being passed into the optional constructor. What do you think?

public class LineItemsViewModel
{
    public LineItemsViewModel()
    {
    }

    public LineItemsViewModel(IPayment payment)
    {
        LineItemColumnHeaders = payment.MerchantContext.Profile.UiPreferences.LineItemColumnHeaders;
        LineItems = LineItemDomainToViewModelMapper.MapToViewModel(payment.LineItems);
        ConvenienceFeeAmount = payment.ConvenienceFee.Fee;
        SubTotal = payment.PaymentAmount;
        Total = payment.PaymentAmount + payment.ConvenienceFee.Fee;
    }

    public IEnumerable<Dictionary<int, string>> LineItems { get; set; }
    public Dictionary<int, string> LineItemColumnHeaders { get; set; }
    public decimal SubTotal { get; set; }
    public decimal ConvenienceFeeAmount { get; set; }
    public decimal Total { get; set; }
}

Hoist

If Total is a domain concept (not just a field shown in a view), you should but the Total = payment.PaymentAmount + payment.ConvenienceFee.Fee logic inside a property or method on your domain model. Business logic doesn't belong in the view model. – Vaginal

Answer 1

Your friend is right. Views should be dumb and not know anything about your domain model.

Have you tried using Automapper to map your business/domain entities/models to your dto/viewmodels?

More details because of comment:

Putting mapping code in your viewmodels violates the separation of concern, the SOLID single responsibility principal, the MVC pattern, and domain driven design principals. Views have one responsibility, get data to the screen, thats it. IMHO there isn't much to argue about. Its simply a bad idea that violates a lot of core software development principals.

Paradies

I appreciate the answer. I want to argue to draw out the most complete answer. I'm talking about the view model, and a constructor on the view model class at that. Can you give a reason (or more) why a view model object should not do its own mapping (via method or constructor). – Hoist

@Byron I let my DTOs do the mapping internally as an implicit cast from a domain object. Sometimes I use the DTOs as a view model and sometimes I combine mutiple DTOs into a composite VM. (I know this probably isn't going to be popular!) I used to use Automapper but I've come across enough complex scenarios that it's just as fast to write them manually; plus it keeps the projection logic with the DTO/VM. What you choose depends on your scenario. – Vaginal

While the mapping code does have to exist somewhere, placing it inside of the view model constructor couples it to the internal structure of the domain. If for some reason you needed to populate the view model from another source, you'd end up adding another constructor to do so with this method. Coupling aside, jfar is right that it simply boils down to a separation of concerns issue. While putting the mapping code there might feel like a convenient thing to do, why should the view model care where the data comes from? – Chadburn

I'm on board with jfar's response, but then where are we meant to Map from domain to view models? Not in the view model or repository for separation of concerns, nor in the controller for testability. Are we supposed to add yet another layer for the mapping of domain to view models? – Wench

@DerekGreer - your comment makes perfect sense to me, however I'm trying to convince a friend which keeps saying give me one practical bad implication of having multiple constructors in the view model/violating separation of concerns here. I told him it violates SRP but he's still saying - "so what if it breaks a theoretic principle? give me one bad situation that might happen that will justify adding another mapping layer instead of doing it in constructors to the view mode"? Can you please give me a concrete example that might convince him? – Welladvised

@Welladvised I'll leave a new answer for this question as I suspect the comments section will be too limiting to accommodate the answer to your question. – Chadburn

Answer 2

This is a rather old question for which an answer has already been accepted, but to elaborate upon the provided answer and to address questions that have come up in the comments section, this additional answer is being provided.

As posed, the question is a bit indefinite. When asked whether something should be done a certain way with respect to software development, the question could be understood to be a question concerning the underlying design principles that govern the topic in question, when such design principles should be applied if not uniformly, or both. To aid in discussing the topic objectively, let's consider these two aspects in turn.

The particular practice of creating a constructor to map the values from one object to another creates a coupling between the two objects. A view model contains the properties and/or the behavior pertaining to a particular view within the system. Since the purpose of such an object is to model a particular view, encapsulating the mapping logic for initializing the internal state/values of the model from another type within the system means that the view model now contains code which may need to be modified for reasons other than changes to how the view is modeled. Such changes open up the possibility that other aspects of the model's behavior may be adversely affected, thus causing unintended regression in the behavior of the system. The principle governing the decoupling of components within the system to prevent unintended regression of behavior by coupling multiple concerns is called The Single Responsibility Principle.

The question of when such principles should be applied is a bit more difficult. It's important to keep in mind that software is typically written with some goal in mind (e.g. solving some business problems, facilitating entertainment or education, etc.) and what is best for any given piece of software is relative to the task at hand. The choices made for a software system that's being created to serve as the flagship product for a particular company may be quite different than the choices made for a system being developed to solve an immediate problem. The scope of work required in order to facilitate certain types of decoupling also need to be considered. Some decoupling techniques are relatively easy to incorporate while others may be more difficult and even come with step learning curves for the initial implementation as well as for each new developer added to the team responsible for the software's maintenance. While no heuristic is perfect for making such decisions, Test-Driven Development sets forth the heuristic of not introducing abstractions until duplication is present. For example, the strategy pattern is an excellent technique for adhering to the Open/Closed Principle, a principle governing the design of objects to allow for their application in different scenarios without needing to modify the existing code. When following Test-Driven Development practices, however, one wouldn't introduce the strategy pattern until a second use-case was observed. By following this heuristic, developers are forced to restrict their efforts to the task at hand by only writing the code necessary to accomplish the task without duplication, resulting in a minimization of waste and maximizing of maintainability (by means of minimizing complexity).

Nevertheless, software engineering is both a science and an art. It's a science in that there are rules which govern what can and can't be done to achieve certain ends, but it's also an art in that you get better at it the more you do it and there are definite trade-offs to be made which ultimately must be made subjectively. For example, as a client software developer, I am typically never involved in the design and development of applications which have a short lifespan. As such, I don't wait until I see duplication before introducing convention-based dependency injection into my applications. Introducing the consistent use of dependency injection within an application has a far lower cost at the beginning of a software system's life than it does waiting until you begin to feel the need for it.

With respect to the specific example of adding mapping code in view models, while it does couple the view model to a particular domain model, in practice I wouldn't find this to be that big of an issue. The view model isn't likely to be used with other domain models and the nature of the type of code being introduced (i.e. mapping) doesn't typically contain business logic, so the likelihood of this SRP violation causing significant regression in the system is far less than an SRP violation at the application or domain layers.

That said, I don't find the process of adding mapping logic within constructors to be any sort of significant time saver. If one were to create a separate class to encapsulate the mapping between the domain object and the view model in most languages, we're only talking about an extra few lines of code. Here's the difference in implementation:

// constructor
public ViewType(DomainType domainType) {
...
}


// mapper class
public class ViewTypeMapper { 
  public ViewType Map(DomainType domainType) {
  ...
  }
}

So, you're either doing a return new ViewType(domainType), or you're doing a return new ViewTypeMapper().Map(domainType). I just don't see where decoupling in this case adds any significant work. In most cases, you've already wasted your company's or client's time and money by even having a discussion about it because you'll invariably end up talking about it for a longer period of time than if you were to just create separate classes to represent the mappings, or if you were to go ahead and just set up Automapper.

Chadburn

Your answer is a masterpiece. Could you please give an example for a change that would adversely affect the view model if the mapping code was inside the constructor, but would of been safe if the mapping is done in a dedicated mapping layer? – Welladvised

Consider if two use cases with differing application-layer APIs resulted in the return of the same view model (e.g. view order, cancel order) and that there was some complexity in the mapping logic (e.g. some LINQ filtering, date conversion, etc.). You may have the first use case working correctly, but having to modify the mapping code for some unique needs of the second case has the potential to break the first. This is the type of regression that SRP helps to avoid. – Chadburn

Additionally, such mapping code adds unnecessary complexity. When a developer examines a particular module of code, all the code they encounter should be relevant to the case in which they are trying to understand ideally. When a class has multiple responsibilities, understanding how the code works requires a developer to understand code that isn't actually relevant to the use case they are working on (i.e. SRP violations adds cognitive load). Of course, these types of issues are less likely to occur or be of significant impact for mapping concerns. I point them out only as general examples. – Chadburn

Thank you for your responses. Regarding your first comment I still didn't get it - if the mapping would be done in orderVMMapper class (forget automapper for now) - I would innocently modify that class for the 'cancel order' needs, but that would still break 'view order', because I would initially write view order's controller to use the same orderVMMapper (to comply with DRY). What am I missing? – Welladvised

The scenario I've described involves mapping the results of two different application level responses (i.e. differing application-level APIs) to the same view model. – Chadburn

OK, so if I understand correctly you meant a scenario like class CanceledOrderDomainModel : BaseOrder, class ActiveOrdersDomainModel : BaseOrder 1.Mapping in constrcutor: class MyViewModel { public MyViewModel(BaseOrder order) { // mapping here } } vs 2.Mapping in a dedicated layer: class BaseOrderMapper { public MyViewModel Map(BaseOrder order) {// mapping here} } But still I'll probably map both API results inside the same Map method in my mapping class (to comply with DRY) so modification in one might break the other. So how did a dedicated layer would make my changes any safer? – Welladvised

That wouldn't serve as a good example given you are saying the differences between your two "domain model" types are irrelevant. What I'm referring to is something like a CancelOrderResponse and an PlaceOrderResponse which contain different representations of the same data which both need to be mapped onto the same OrdersViewModel. Given the mapping logic is non-trivial, changes for one use case could inadvertently affect the other use case. – Chadburn

Thank you for the clarification. If I have a ActiveOrders property on my view model. And 2 mapping constructors. One does:ActiveOrders = cancelResponse.Orders.Where(o=>o.IsActive) the other does:ActiveOrders = placeOrderResponse.Orders.Where(o=>o.IsActive && o.Approved). How changes in one constructor could affect the other? (I understand that I could be tempted to modify shared ActiveOrders property for a specific use case because I'm already inside that class, but it seems you refer to a change inside the mapping constructor itself? Could you please provide an example of such a change? – Welladvised

The scenario you are describing would likely not lead to issues unless you accidentally modified the wrong constructor in haste, affected the wrong constructor through some sort of automatic refactoring, etc.. The potential for issues increases if the complexity of the mapping results in factoring out common aspects of the mapping into separate methods. As I've stated, this particular type of SRP violation isn't going to be as egregious as the same sort of violation in a class with business logic. – Chadburn

The real point comes down to this: writing a separate mapping class is almost zero extra effort, is a consistent approach with the design principles we hope are governing the rest of the application, reduces the noise, and is an approach that reinforces good coding practices. If we were talking about something with some actual trade off value then that would be a different conversation. In cases where there is no perceived advantage, my recommendation is to allow yourself to be guided by design principles. Their benefit is often realized in the cases you can't foresee. – Chadburn

Good answer! Here is my opinion. If you create a constructor for each kind of domainType, then you can see every possible way that the viewmodel can / should be populated just by looking at its constructors. If you want to change the view and corresponding viewmodel, then you will need to change all of the ways to populate the viewmodel, and this is easier if it's all in the viewmodel constructors. This seems to me to be much more valuable and maintainable than adhering to a strict SRP. – Gautea

I should have also included in the article that adding a constructor each time a new mapping needs to be made is also an example of violating the Open/Closed principle. Again though, view models in particular aren't particularly prone to regression because they generally don't contain behavior and aren't generally injected with an IoC container. For other object-role stereotypes, I wouldn't recommend this practice though and when the majority of cases benefit from decoupling and it isn't costly, I'd favor consistency (i.e. just keep the mapping external for all types). – Chadburn