9

I'm currently working as a solo developer on my current project. I inherited the project from another developer, who has since left the company. It's a model-view-controller style web application in C#. It uses Entity Framework for object relational mapping. And there are two different sets of classes for the types in the domain model. One set is used for interacting with the ORM and the other is used as the models in the MVC system. For example, there might be two classes as follows:

public class Order{
    int ID{get;set;}
    String Customer{get;set;}
    DateTime DeliveryDate{get;set;}
    String Description{get;set;}
}

and

public class OrderModel{
    String Customer{get;set;}
    DateTime DeliveryDate{get;set;}
    String Description{get;set;}
    public OrderModel( Order from){
        this.Customer= from.Customer;
        // copy all the properties over individually
    }
    public Order ToOrder(){
        Order result =new Order();
        result.Customer = this.Customer;
        // copy all the properties over individually
    }

}

I can think of several drawbacks to this approach (more places to change code if something changes, more objects sitting around in memory, more time spent copying data around), but I'm not really sure what the advantages are here. More flexibility for the model classes, I suppose? But I could get that by subclassing the entity classes as well. My inclination would be to merge these two class groups, or possibly have the model classes be subclasses of the entity classes. So am I missing something important here? Is this a common design pattern I'm not aware of? Are there good reasons not to go through with the refactor I'm contemplating?

UPDATE

Some of the answers here are making me realize my initial description of the project was lacking some important details. There is also a third group of classes that exist in the project: the page model classes. They are the ones actually being used as the model backing the page. They also contain information that is specific to the UI, and would not be stored with an order in the database. An example page model class might be:

public class EditOrderPagelModel
{
    public OrderModel Order{get;set;}
    public DateTime EarliestDeliveryDate{get;set;}
    public DateTime LatestAllowedDeliveryDate{get;set;}
}

I totally see the utility of this third group being distinct here, and have no plans to be merging it with something else (though I might rename it).

The classes in the model group are also currently used by the application's API, which I'd also be interested in hearing input on whether that is a good idea.

I should also mention that the customer being represented as a string here was to simplify the example, not because it's actually being represented that way in the system. The actual system has the customer as being a distinct type in the domain model, with its own properties

  • 1
    "I inherited the project from another developer, who has since left the company" there is a site dedicated to stories that begin this way. – user22815 May 4 '15 at 15:05
  • Regarding your update: that seems like too much indirection for not enough benefit. – Robert Harvey May 4 '15 at 18:26
  • @RobertHarvey What are you referring to as being too much indirection? – Robert Ricketts May 4 '15 at 18:40
  • 1
    The OrderModel. OrderViewModel (what your are probably referring to as the EditOrderPageModel) is sufficient indirection; see my answer below. – Robert Harvey May 4 '15 at 19:48
  • @RobertHarvey This sounds like the answer to the question I was actually trying to ask – Robert Ricketts May 4 '15 at 21:42
16

So am I missing something important here?

YES

While these look like the same thing, and represent the same thing in the domain, they are not the same (OOP) objects.

One is the Order as known by the data storage part of the code. The other is the Order as known by the UI. While it's a pleasant coincidence that these classes have the same properties, they're not guaranteed to.

Or to take a look at it from another perspective, consider the Single Responsibility Principle. The key motivator here is that "a class has one reason to change". Especially when dealing with pervasive frameworks like an ORM and/or UI framework, your objects need to be well isolated since changes to the framework will often cause your entities to change.

By tying your entities to both frameworks, you're making it that much worse.

7

The purpose of the View Model is to provide decoupling in two ways: by providing data in the shape that the View requires (independent of the model), and (especially in MVVM) by pushing some or all of the View logic back from the View to the View Model.

In a fully normalized model, Customer would not be represented in the Model by a string, but rather by an ID. The Model, if it needs the name of the customer, would look up the name from the Customers table, using the CustomerID found in the Orders table.

A View Model, on the other hand, may be more interested in the Name of the customer, not the ID. It has no need for the ID unless the Customer is being updated in the Model.

In addition, the View Model can, and usually does, take a different shape (unless it is pure CRUD). An Invoice View Model object might have customer name, addresses and line items, but the model contains customers and descriptions.

In other words, invoices are normally not stored in the same way they are displayed.

2

In some respect you are right: They both refer to the same domainobject, which is called order. But you are wrong in as far, it is not a real duplication than a different representation - originated from different purposes. If you want to persist your order, you e.g. want access to each and every column. But you do not want to leak that to the outside. Besides pushing whole Entities through the wire is not what you really want. I know of cases, where a collection of MBs of orders was shipped to the client where a few kb would be enough.

To make a long story short - here are the main reasons you want to do that:

1) Encapsulation - Information hiding of your application

2) Small Models are fast to serialize and easy to transmit

3) They serve different purposes, although the are overlapping, and therefore there should be different objects.

4) Sometimes it makes sense to have for different Queries different Value-Objects. I do not know how it is in C#, but in Java it is possible to use a POJO to collect results. If I need a single order I use the Order-Entity, but if I only need some columns filled with amounts on data, using smaller Objects is more efficient.

  • The way that entity framework is structured, entities are plain old C# objects, so sending them over the wire isn't really much of a problem. I do see the utility of being able to send just part of the content of an order around in some circumstances. – Robert Ricketts May 4 '15 at 22:00
  • One or a bunch is not the problem as I said. But there are cases, where you have MBs of data, which slows your loadtime down. Think of mobile load times – Thomas Junk May 5 '15 at 4:42
0

(Disclaimer: I only saw it being used in this way. I might have misunderstood the real purpose of doing so. Treat this answer with suspicion.)

Here is another missing bit: the conversion between Order and OrderModel.

The Order class is tied to your ORM, while the OrderModel is tied to the design of your View Model.

Typically, the two methods will be provide "in a magical way" (sometimes called DI, IoC, MVVM, or yet another thing). That is, instead of implementing these two conversion methods as part of OrderModel, they will instead belong to a class dedicated to inter-conversion of these things; that class will be somehow registered with the framework so that the framework can easily look it up.

public class OrderModel {
    ...
    public OrderModel(Order from) { ... }
    public Order ToOrder() { ... }
}

The reason for doing this is that whenever there is a crossing from the OrderModel to the Order or vice versa, you know that some data must have been loaded from or saved into ORM.

  • If I am keeping both around, I definitely would like to set them up to have the conversion be more automated – Robert Ricketts May 4 '15 at 17:47
  • @RobertRicketts The thing I saw looks like this: IMvxValueConverter. Though I may have misunderstood its purpose. – rwong May 4 '15 at 18:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.