CODEWITHSUNDEEP
asp.net-mvc
Thiago
Thiago

Reputation: 1565

Leaner controller

Hy guys.

I am new on MVC and I have a "fat" controllers and I don't know how to fit it.

This is one controller where I create a new repository and then a ViewModel gets the repo values + the isReaded property

      public ActionResult Index()
    {
        try
        {

            NHibernateHelper helper = new NHibernateHelper();
            UnitOfWork unitOfWork = new UnitOfWork( helper.SessionFactory );
            Repository<Order> orderRepo = new Repository<Order>( unitOfWork.Session );
            IEnumerable<Order> orders = orderRepo.All();

               var viewModel = orders.Select(order=> new OrderViewModel
                {
                    Order = order,
                    isReaded = order.Interactions.Any( x => x.Readed == true ),
                } );

                return View( viewModel );


        }
        catch
        {
            return RedirectToAction( "foo");
        }
    }

Can someone give me a tip to fit it?

Tks!

Upvotes: 0

Views: 104

Answers (2)

smartcaveman
smartcaveman

Reputation: 42256

Use dependency injection and refactor these lines to assign private readonly fields in your controller's constructor.

        NHibernateHelper helper = new NHibernateHelper();
        UnitOfWork unitOfWork = new UnitOfWork( helper.SessionFactory );
        Repository<Order> orderRepo = new Repository<Order>( unitOfWork.Session );

Then, refactor the try-catch statement to an exception handling action filter. A tutorial and code for this is available at http://www.squaredroot.com/2008/04/02/mvc-error-handler-filter/ .

Then your controller action is down to.

  [RedirectToActionOnError(Action = "foo")]
  public ActionResult Index()
  {
        var viewModel = _orderRepo.All()
            .Select(order=> new OrderViewModel
            {
                Order = order,
                isReaded = order.Interactions.Any(x => x.Readed),
            });
         return View(viewModel);
 }

As a side note, you never have to write if(someBooleanValue == true), you can just write if(someBooleanValue)

Upvotes: 2

Tejs
Tejs

Reputation: 41256

Assigning data to your view is always going to be a Controller responsibility; that said, it seems you could abstract your logic into a class that handles using your repository and returning back a simple object that has the details you need.

var myModelData = _someOtherObject.GetSelectedOrder();

var viewModel = new OrderViewModel
{
    Order = myModelData.Order,
    isReaded = myModelData.IsRead
};

return View(viewModel);

For your exceptions, you should define an ErrorFilterAttribute and apply it to your action method, if you want to just have your _someOtherObject method throw an exception.

Upvotes: 0

Related Questions