asp.net mvc -> the rise of the messy controllers

All the buzz these days goes to asp.net mvc and as many others, I’m loving it!

Messing around with it, and looking at samples of others, I see the controllers tend to get quite messy. Where normally the codebehinds got messy, it’s now moved to the controllers.

Let’s say we’re using Subsonic
as our data-access layer and we have created this controlleraction:

[ControllerAction]
public void List(string foldername)
{
    ICollection folders = GetFolders(userId);
    bool hasInbox = false, hasDrafts = false, hasSent = false, hasBin = false; 
    foreach (PrivateMessageFolder folder in folders) 
    { 
        hasInbox = hasInbox || folder.FolderType == PrivateMessageFolderType.Inbox;
        hasDrafts = hasDrafts || folder.FolderType == PrivateMessageFolderType.Drafts; 
        hasSent =hasSent || folder.FolderType == PrivateMessageFolderType.Sent; 
        hasBin = hasBin|| folder.FolderType == PrivateMessageFolderType.Bin; 
        if (hasInbox && hasDrafts && hasSent && hasBin
            ) break; 
    } 
    if (!hasInbox) 
    { 
        folders.Add(CreateSystemFolder(PrivateMessageFolderType.Inbox, userId));
    } 
    if (!hasDrafts) 
    { 
        folders.Add(CreateSystemFolder(PrivateMessageFolderType.Drafts, userId)); 
    } 
    if (!hasSent) 
    { 
        folders.Add(CreateSystemFolder(PrivateMessageFolderType.Sent, userId)); 
    } 
    if (!hasBin) 
    { 
        folders.Add(CreateSystemFolder(PrivateMessageFolderType.Bin, userId)); 
    } 
    PrivateMessageCollection messages = new PrivateMessageCollection(); 
    messages.LoadAndCloseReader(SPs.GetMessages(foldername, UserId).GetReader());
    PMData data = new PMData
    {
        Folders = folders,
        PrivateMessages = messages
    };
    RenderView("List", data);
}

Now, while this works perfectly fine, it’s messy. It’s messy because now my controller is very dependant on how subsonic works. So if subsonic changes in the future, or I choose to pick another dataaccess layer since subsonic support has stopped(or whatever reason), I have to rewrite all my controllers! Not nice, not nice at all.
It’s also messy because I can’t see at one glance what my controller is passing to my view. I have to actually understand the code to see what it’s passing on.

Now look at this peice of code:

Daos.PrivateMessageDao privateMessageDao = new Forums.Daos.PrivateMessageDao();

[ControllerAction]
public void List(string foldername)
{
    //load the collection
    PMData data = new PMData
    {
        Folders = privateMessageDao.GetAndEnsureFolders(UserId),
        PrivateMessages = privateMessageDao.GetMessages(foldername, UserId)
    };
    RenderView("List", data);
}

All code that has been removed, is now in the privatemessagedao

To me, its quite clear what this is passing on, it’s very clean code. I moved all the data-access parts to the Dao. So the dao (or service, or what ever you want to name it) contains the logic that’s needed to retreive, update, delete, etc. The controller shouldn’t even care how that dao does that, it just needs the POCO‘s.

In our example, we also have folders to be created by default. This is by no means the responsibility of the controller, the controller should just tell something else that it needs those folders, that’s it. The other thingy should create them.

Now we have seperation of concerns, so we have the possibility of using Dependency Injection for accessing the dao (and others), but I’ll write about that some other time