Unity IoC and MVC modelbinding

Question

Is it ok to have a static field in my controller for my modelbinder to call ?

Eg.

public class AuctionItemsController : Controller
{
    private IRepository<IAuctionItem> GenericAuctionItemRepository;
    private IAuctionItemRepository AuctionItemRepository;

    public AuctionItemsController(IRepository<IAuctionItem> genericAuctionItemRepository, IAuctionItemRepository auctionItemRepository) {
        GenericAuctionItemRepository = genericAuctionItemRepository;
        AuctionItemRepository = auctionItemRepository;
        StaticGenericAuctionItemRepository = genericAuctionItemRepository;
    }

    internal static IRepository<IAuctionItem> StaticGenericAuctionItemRepository;

here is the modelbinder

public class AuctionItemModelBinder : DefaultModelBinder
{
    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext) {
        if (AuctionItemsController.StaticGenericAuctionItemRepository != null) {
            AuctionLogger.LogException(new Exception("controller is null"));
        }

        NameValueCollection form = controllerContext.HttpContext.Request.Form;

        var item = AuctionItemsController.StaticGenericAuctionItemRepository.GetSingle(Convert.ToInt32(controllerContext.RouteData.Values["id"]));

        item.Description = form["title"];
        item.Price = int.Parse(form["price"]);
        item.Title = form["title"];
        item.CreatedDate = DateTime.Now;
        item.AuctionId = 1;


        //TODO: Stop hardcoding this
        item.UserId = 1;

        return item;
    }}

i am using Unity as IoC and I find it weird to register my modelbinder in the IoC container.

Any other good design considerations I shold do ?

Answer 1

No, I wouldn't consider this good practice. The beauty about API design in general, and DI in particular, is that as soon as something starts looking weird, it should trigger an mental alert. The static keyword has that effect on me.

Once you start using static properties, there's no reason to get your repository from the Controlller - you might just as well get it directly from the container, and that would imply the Service Locator anti-pattern. As it is now, you tightly couple your ModelBinder to a specific Controller, although there seems to be no reason why you would want to do that.

Technically, you can do what you already do, but consider whether it's the correct place: now that you have your item from the repository, why not save it again immediately? That's also technically possible, but a big violation of the Single Responsibility Principle.

Think about the intended responsibility of a ModelBinder: it is to translate HTTP/HTML contextual information (given in plain text) into a strongly typed object. That's it. If you try to make it do more, you are breaking the SRP.

Dehydration based on HTML Form/querystring values is better left to the Controller itself. Once the Controller has a proper Domain Object, it can hand it off to the Domain Model. That should be the single responsibility of the Controller. In most cases you don't need a ModelBinder at all.