patterncsharpMinor

Support ticket application: ticket listing index with sorting and filtering

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

sortingapplicationwithticketlistingandindexsupportfiltering

Problem

I am new to ASP.NET (C#) and MVC 5. I'm really enjoying it so far, but want to take a step back and make sure that I'm following best practices here. My controller seems like it may be getting too fat, but perhaps that's just part of it?

This is the Index action for the TicketsController in a support ticket application that I'm building to familiarize myself with MVC5. Everything works just fine, but it's starting to smell and I am looking for feedback.

TicketsController -> Index:

```
/// GET /Tickets/
public ActionResult Index(string sortOrder, int? ShopFilter, bool showAll = false)
{
TicketIndexViewModel ticketIndexVM = new TicketIndexViewModel();

ticketIndexVM.SortOrder = sortOrder;
ticketIndexVM.IdSortParam = String.IsNullOrEmpty(sortOrder) ? "id_asc" : "";
ticketIndexVM.ShopNameSortParam = sortOrder == "shop_desc" ? "shop_asc" : "shop_desc";
ticketIndexVM.StatusSortParam = sortOrder == "status_desc" ? "status_asc" : "status_desc";
ticketIndexVM.ShowAllParam = showAll ? "true" : "";
ticketIndexVM.ShopFilterParam = ShopFilter;

ticketIndexVM.ShopListing = Mapper.Map(db.Shops, new List());

var tickets = from t in db.Tickets.Include(t => t.User).Include(t => t.Shop)
select t;

// hide completed and cancelled tickets
if(!showAll)
{
tickets = tickets.Where(t => t.Status s.ShopId == ShopFilter);

if (shopById.Count() > 0)
{
tickets = tickets.Where(t => t.ShopId == ShopFilter);
}
else
{
// that shop doesn't exist.
ticketIndexVM.ShopFilterParam = null;
ticketIndexVM.Errors.Add(
String.Format("No shop exists with the ID# {0}", ShopFilter));
}
}

// handle sorting
switch (sortOrder)
{
case "id_asc":
tickets = ticke

Solution

I think methods for interacting with db classes, manipulating and filtering data
belong in the model.

So if there is already an appropriate model class for the controller then move the code there or create a new model to service the controller.

I would create a model TicketsService and a method inside it to return tickets; something like the following -

public class TicketService 
{

public TicketIndexViewModel getTickets (string sortOrder, int? ShopFilter,     bool showAll = false) {

TicketIndexViewModel ticketIndexVM = new TicketIndexViewModel();

    ticketIndexVM.SortOrder = sortOrder;
    ticketIndexVM.IdSortParam = String.IsNullOrEmpty(sortOrder) ? "id_asc" : "";
    ticketIndexVM.ShopNameSortParam = sortOrder == "shop_desc" ? "shop_asc" : "shop_desc";
    ticketIndexVM.StatusSortParam = sortOrder == "status_desc" ? "status_asc" : "status_desc";
    ticketIndexVM.ShowAllParam = showAll ? "true" : "";
    ticketIndexVM.ShopFilterParam = ShopFilter;

    ticketIndexVM.ShopListing = Mapper.Map(db.Shops, new List());

    var tickets = from t in db.Tickets.Include(t => t.User).Include(t => t.Shop)
                  select t;

    // hide completed and cancelled tickets
    if(!showAll)
    {
        tickets = tickets.Where(t => t.Status  s.ShopId == ShopFilter);

        if (shopById.Count() > 0)
        {
            tickets = tickets.Where(t => t.ShopId == ShopFilter);
        }
        else
        {
            // that shop doesn't exist.
            ticketIndexVM.ShopFilterParam = null;
            ticketIndexVM.Errors.Add(
                String.Format("No shop exists with the ID# {0}", ShopFilter));
        }
    }

    // handle sorting
    switch (sortOrder)
    {
        case "id_asc":
            tickets = tickets.OrderBy(t => t.TicketId);
            break;
        case "shop_desc":
            tickets = tickets.OrderByDescending(t => t.Shop.Name);
            break;
        case "shop_asc":
            tickets = tickets.OrderBy(t => t.Shop.Name);
            break;
        case "status_desc":
            tickets = tickets.OrderByDescending(t => t.Status);
            break;
        case "status_asc":
            tickets = tickets.OrderBy(t => t.Status);
            break;
        default:
            tickets = tickets.OrderByDescending(t => t.TicketId);
            break;
    }

    var ticketsVM = Mapper.Map(tickets, new List());

    ticketIndexVM.Tickets = ticketsVM;

    return ticketIndexVM; 

}

}

And my controller method would look something like -

public ActionResult Index(string sortOrder, int? ShopFilter, bool showAll = false)
{

return View(new TiceketService().getTickets(sortOrder,ShopFilter,showAll));
}

On a side note : You might want to look into the dependency injection design pattern. I have been following it in my project and it has worked well for me.

You can read more about it here

http://www.asp.net/mvc/overview/older-versions/hands-on-labs/aspnet-mvc-4-dependency-injection

Hope this helps.

Code Snippets

public class TicketService 
{

public TicketIndexViewModel getTickets (string sortOrder, int? ShopFilter,     bool showAll = false) {

TicketIndexViewModel ticketIndexVM = new TicketIndexViewModel();

    ticketIndexVM.SortOrder = sortOrder;
    ticketIndexVM.IdSortParam = String.IsNullOrEmpty(sortOrder) ? "id_asc" : "";
    ticketIndexVM.ShopNameSortParam = sortOrder == "shop_desc" ? "shop_asc" : "shop_desc";
    ticketIndexVM.StatusSortParam = sortOrder == "status_desc" ? "status_asc" : "status_desc";
    ticketIndexVM.ShowAllParam = showAll ? "true" : "";
    ticketIndexVM.ShopFilterParam = ShopFilter;

    ticketIndexVM.ShopListing = Mapper.Map(db.Shops, new List<ShopViewModel>());

    var tickets = from t in db.Tickets.Include(t => t.User).Include(t => t.Shop)
                  select t;

    // hide completed and cancelled tickets
    if(!showAll)
    {
        tickets = tickets.Where(t => t.Status < 3);
    }

    // shop filtering
    if(ShopFilter != null)
    {
        var shopById = db.Shops.Where(s => s.ShopId == ShopFilter);

        if (shopById.Count() > 0)
        {
            tickets = tickets.Where(t => t.ShopId == ShopFilter);
        }
        else
        {
            // that shop doesn't exist.
            ticketIndexVM.ShopFilterParam = null;
            ticketIndexVM.Errors.Add(
                String.Format("No shop exists with the ID# {0}", ShopFilter));
        }
    }

    // handle sorting
    switch (sortOrder)
    {
        case "id_asc":
            tickets = tickets.OrderBy(t => t.TicketId);
            break;
        case "shop_desc":
            tickets = tickets.OrderByDescending(t => t.Shop.Name);
            break;
        case "shop_asc":
            tickets = tickets.OrderBy(t => t.Shop.Name);
            break;
        case "status_desc":
            tickets = tickets.OrderByDescending(t => t.Status);
            break;
        case "status_asc":
            tickets = tickets.OrderBy(t => t.Status);
            break;
        default:
            tickets = tickets.OrderByDescending(t => t.TicketId);
            break;
    }

    var ticketsVM = Mapper.Map(tickets, new List<TicketViewModel>());

    ticketIndexVM.Tickets = ticketsVM;

    return ticketIndexVM; 



}

}

public ActionResult Index(string sortOrder, int? ShopFilter, bool showAll = false)
{

return View(new TiceketService().getTickets(sortOrder,ShopFilter,showAll));
}

Context

StackExchange Code Review Q#96262, answer score: 3

Revisions (0)

No revisions yet.