Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I need help refactoring this code to dynamically generate a sort string that I can send to my data layer so that my database does the sorting instead of it happening in memory. I am using MVC4 with EF5 as my data layer.

public ActionResult InstanceSearch(int? page, string sortOrder, string computerName, string instanceName,
                                       string productName, string version)
        int pageSize = 10;

        int pageNumber = (page ?? 1);
        if (pageNumber < 1)
            pageNumber = 1;

        if (string.IsNullOrEmpty(sortOrder))
            sortOrder = "Computer";

        ViewBag.CurrentSort = sortOrder;
        ViewBag.ComputerSort = sortOrder == "Computer" ? "ComputerDesc" : "Computer";
        ViewBag.InstanceSort = sortOrder == "Instance" ? "InstanceDesc" : "Instance";
        ViewBag.VersionSort = sortOrder == "Version" ? "VersionDesc" : "Version";
        ViewBag.ProductSort = sortOrder == "Product" ? "ProductDesc" : "Product";

        //IEnumerable<Instance> instances = ecuWebDataContext.Instances;

        IQueryable<Instance> instances
        = string.IsNullOrEmpty(computerName) == true ? ecuWebDataContext.Instances : ecuWebDataContext.Instances.Where(i => i.Computer.Name == computerName);

        if (string.IsNullOrEmpty(instanceName) == false)
            instances = instances.Where(i => i.Name == instanceName);

        if (!string.IsNullOrEmpty(productName))
            ProductName product = (ProductName)Enum.Parse(typeof(ProductName), productName);
            instances = instances.Where(i => i.Product.Name == product);

        if (!string.IsNullOrEmpty(version))
            instances = instances.Where(i => i.Version == version);

        switch (sortOrder)
            case "Computer":
                instances = instances.OrderBy(i => i.Computer.Name);

            case "ComputerDesc":
                instances = instances.OrderByDescending(i => i.Computer.Name);

            case "Instance":
                instances = instances.OrderBy(i => i.Name);

            case "InstanceDesc":
                instances = instances.OrderByDescending(i => i.Name);

            case "Version":
                instances = instances.OrderBy(i => i.Version);

            case "VersionDesc":
                instances = instances.OrderByDescending(i => i.Version);

            case "Product":
                instances = instances.OrderBy(i => i.Product.Name);
                //instances = instances.OrderBy(i => Enum.Parse(typeof(ProductName), i.Product.Name));

            case "ProductDesc":
                instances = instances.OrderByDescending(i => i.Product.Name);
                //instances = instances.OrderByDescending(i => Enum.Parse(typeof(ProductName), i.Product.Name));
        ViewBag.SortOrder = sortOrder;

        var instanceSearchModel = new InstanceSearchModel { ComputerName = computerName, InstanceName = instanceName };
        ViewBag.ComputerName = computerName;
        ViewBag.InstanceName = instanceName;
        ViewBag.ProductName = productName;
        ViewBag.InstanceCount = instances.Count();
        ViewBag.Version = version;
        return View(instances.ToPagedList(pageNumber, pageSize));

I tried returning an IQueryable from the data layer, but then I run into connection problems because I never close it.

share|improve this question
Does this work correctly? If not, then you need to get it working before posting here. – Leonid Jan 4 '13 at 3:13
Yes this works correctly. However I have wrappers around the datalayer to hide it behind interfaces so I can swap out databases without affecting the application and this code uses the database context directly which has the intened effect of offloading to the database for the sorting, but I am looking for help building the sort string then sending it completely to my data layer. – twreid Jan 4 '13 at 16:53
up vote 1 down vote accepted

The data layer that appears to be causing your problem is the root cause.

You're abstracting over an abstraction, and in the process having to marshall sort orders around as a result. Prefer injecting your context in a request scope and remove the abstraction.

You can always create an abstraction over the specific query, and inject the query, which has a dependency on the context, if you really need to.

share|improve this answer
Ok thank you i will get started on doing that. – twreid Jan 11 '13 at 15:47

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.