Using Service layer classes in ViewModels. A Design Flaw?

Question

I am wondering if I have a design flaw in my solution. Here's what I have:

• Entities => pure poco. refs: none.
• Data => data access. refs: Entities.
• Service => business logic. refs: Entities and Data
  • Manager Classes.
  • ViewModels.
• WebApp => UI. Refs: Entities and Service

WebApp ASP.NET MVC project as UI, Entities project with no reference for holding pure POCO. Data to access database and Service for Business Logic (where my Manager classes are).

Basically, I have defined a Manager class for every entity. For exmaple, I have a Message entity with relation to a list of Recipient entity. I have a MessageManager and RecipientManager class, responsible for both CRUD operations using Data layer and logical results (e.g. public List<Message> GetAllMessagesWithPermissionForUser(User user, Permission permission))

For my MVC project I have defined some ViewModel classes in Service layer to generate specific videmodels for my views. Since the viewmodels are using Manager classes, I have defined them in my Service Class. For example I have a MessageOperationVM viewmodel which has a PermittedBoxesToSend property. This property uses my BoxManager class to get all boxes permitted for the specified message:

// Initialized by Catsle Windsor.
public BoxManager BoxManager {get; set;}

public List<Box> PermittedBoxesToSend 
{
    if(this._premittedBoxesToSend != null)
    {
        this._permittedBoxesToSend = BoxManager.GetPermittedBoxesToSend(this.Message);
    }
}

• I am not sure if using manager classes in Viewmodels are a good design. Although I've defined them as constructor/property setters to get populated with DI. Should I populate my viewmodels' properties in my controllers instead of defining properties and get rid of manager classes in my ViewModels?

  public ActionResult ShowNewMessageDialog()
  { 
      var messageVM = new MessageOperationVM() { new Message() };
      messageVM = this.BoxManager.GetPermittedBoxesToSend();
  }
  

• using a manager class for each entity seems to make maintenance hard. (Although they all derive from a BaseManager class which shares the operations they have in common)

• Is there any reconsideration worth mentioning in the above design?

Thank you.

Update:
based on eulerfx's answer:
The problem I have with your answer is: To construct a ViewModel, I have to call some service layer's methods. So I cannot construct my ViewModel just based on my poco entity. Do you suggest that I should construct those parts in controller as well? :

public ActionResult ShowNewMessageDialog()
{
    var message = this.messageRepository.GetMessage();
    var messageVM = new MessageViewModel(message);
    messageVM.CustomProperty = this.messageManager.CallSomeMethod(message);
    return View(messageVM);
}

Answer 1

I would consider references from view models to services a design flaw. View models should be flat and simple DTOs purposed toward binding with the view. They should not be part of a DI container graph because that complicates things and makes reasoning about the code more difficult. The accepted definitions for the terms you are using are as follows.

• Entites are entities as you described. They are simple POCO classes.
• What you call "Data" is the repository pattern. The repository provides access to and persistence of entities in an underlying database.
• Service is an overloaded term, however they are generally used to encapsulate the domain exposing it as an API to other application layers. Services can reference repositories. I wrote a blog post about these types of services in the context of DDD here where they are called application services.
• View models are part of the presentation layer or the WebUI as you called it. As such, they are constructed by the MVC controller and passed to the view. The controller constructs the view model using data obtained from an application service or directly from a repository. The view model can contain a constructor which accepts an entity returned by the service or repository.

The code can look something like this:

/// Domain model class that lives in domain/business layer project
public class Message
{
  // properties and behavior go here
}

// View model class that lives in the ASP.NET project
public class MessageViewModel
{
  public MessageViewModel() { }
  public MessageViewModel(Message message)
  {
    // construct the view model based on the provided entity
  }
  // properties specific to the view go here
} 


// ASP.NET MVC controller.
public class MessagesController : Controller
{
 // this repository should be injected by DI container.
 readonly IMessageRepository messageRepository;

 public ActionResult ShowNewMessageDialog()
 {
   var message = this.messageRepository.GetMessage();
   return View(new MessageViewModel(message));
 }
}

Answer 2

Instead of the Manager classes, you may consider using a Visitor Design Pattern to manipulate the Entities and Data classes.

If the Manager or Visitor classes are defined in such that they dont have to be used only in the View code, then it shouldnt be a problem. But if they can only be used in the View, and for other Views, etc you have to redefine the access to the Entities and Data classes, or write more/duplicate code, then that's problematic.