Handling conditional logic inside controller actions

I am looking for best practice in handling conditions inside the controller actions in ASP.NET MVC:

public ActionResult Edit(int Id = 0)
{
   var Item = _todoListItemsRepository.Find(Id);
   if (Item == null)
      return View("NotFound");
   if (!Item.IsAuthorized())
      return View("NotValidOwner");

   return View("Edit", Item);
}

The above two ifs are used in other actions inside the controller. So, in order not to repeat these conditions in all the actions. I have used the below approach:

[HttpGet]       
[Authorize]
[ModelStatusActionFilter]
public ActionResult Edit(int Id = 0)
{
    var Item = _todoListItemsRepository.Find(Id);        
    return View("Edit", Item);
}

public class ModelStatusActionFilterAttribute : ActionFilterAttribute
{
    private readonly ITodoListItemsRepository _todoListItemsRepository;
    public ModelStatusActionFilterAttribute()
        : this(new TodoListItemsRepository())
    {

    }
    public ModelStatusActionFilterAttribute(ITodoListItemsRepository     todoListItemsRepository)
    {
        _todoListItemsRepository = todoListItemsRepository;
    }
    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        try
        {
            var Id = Convert.ToInt32(filterContext.RouteData.Values["Id"]);
            var Item = _todoListItemsRepository.Find(Id);
            if (Item == null)
            {
                filterContext.Result = new ViewResult() { ViewName = "NotFound" };
            }
            else if (!Item.IsAuthorized())
            {
                filterContext.Result = new ViewResult() { ViewName = "NotValidOwner" };
            }
        }
        catch
        {

        }
    }                
}

I am unsure if this is the best practice in handling such scenarios. Could someone please advise?

This is just copy of my answer to your question on SO.

Usually you don't use action filters for so-called business logic of your web application - this is what the controllers are for. Action filter are rather for the whole stuff which is external to the actual logic - common case is logging, performance measurement, checking if user is authenticated / authorized (I don't think this is your case, although you call IsAuthorized method on the "Item").

Reducing code is generally good thing but in this case, I don't think putting the logic to action is a good way, because you've actually made it a bit unreadable, and unreadable code is in my opinon much worse than repeated code. Also, specifically in your case, for all valid items you actually call the _todoListItemsRepository.Find() twice (for each valid item), which might be costly if this is some webservice call or db lookup.

If the code is just repeated throughout the actions, make a method out of it like:

private View ValidateItem(Item) {
    if (Item == null)
      return View("NotFound");
   if (!Item.IsAuthorized())
      return View("NotValidOwner");
return null; }

StackExchange Code Review Q#75187, answer score: 3