6

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; }
}
Improve this question
1
  • 1
    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. Commented May 18, 2010 at 19:03

2 Answers 2

Reset to default
7

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

12 Comments

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.
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.
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.
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.
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.
|
6

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.

Improve this answer

6 Comments

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).
@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.
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?
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?
@BornToCode 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.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.