-1
\$\begingroup\$

Is there any way I can refactor this expression tree?

    public static IOrderedQueryable<T_PO> SortedPO(this IQueryable<T_PO> query,  ExtjsGridPagingParameter.SortingParameter sortingParameter)
    {

        IOrderedQueryable<T_PO> result = null;

        if (String.IsNullOrEmpty(sortingParameter.property) == false)
        {                
            Expression<Func<T_PO, string>> orderField = null;
            Expression<Func<T_PO, DateTime?>> orderFieldDate = null;
            Expression<Func<T_PO, decimal>> orderFieldDecimal = null;
            switch (sortingParameter.property)
            {
                case "po_code":

                    orderField = (item => item.po_code);
                    break;

                case "proj_name":

                    orderField = (item => item.T_PROJECT.proj_name);
                    break;

                case "po_supplier_contact":

                    orderField = (item => item.po_supplier_contact);
                    break;

                case "po_grandtotal":

                    orderFieldDecimal = (item => item.po_grandtotal);
                    break;

                case "po_docdate":

                    orderFieldDate = (item => item.po_docdate);
                    break;

                default:
                    break;
            }


            if (orderField != null)
            {
                if (sortingParameter.direction == "ASC")
                {
                    result = query.OrderBy(orderField);
                }
                else
                {
                    result = query.OrderByDescending(orderField);
                }
            }
            else if (orderFieldDate != null)
            {
                if (sortingParameter.direction == "ASC")
                {
                    result = query.OrderBy(orderFieldDate);
                }
                else
                {
                    result = query.OrderByDescending(orderFieldDate);
                }
            }
            else if (orderFieldDecimal != null)
            {
                if (sortingParameter.direction == "ASC")
                {
                    result = query.OrderBy(orderFieldDecimal);
                }
                else
                {
                    result = query.OrderByDescending(orderFieldDecimal);
                }
            }


        }

        return result;
    }
\$\endgroup\$

closed as unclear what you're asking by RubberDuck, Edward, Mast, Jamal Jan 16 '16 at 22:25

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. For examples of good titles, check out Best of Code Review 2014 - Best Question Title Category You may also want to read How to get the best value out of Code Review - Asking Questions. \$\endgroup\$ – Caridorc Oct 5 '15 at 13:32
  • \$\begingroup\$ Okay. It's got something to do with expression trees. Got it, but what is this code supposed to do? You've not given us enough context to properly review this code. What is its purpose? What are the expected inputs & outputs? Do you have any particular concerns about the code? If so, what are they? \$\endgroup\$ – RubberDuck Jan 16 '16 at 12:23
6
\$\begingroup\$

Note that your code will return null in case if the sort order is not defined. I've preserved this logic, but you may want to change it to return original query (and thus change return type to IQueryable<T_PO>).

You are doing 2 separate actions in this method, so if you split it into 2 parts you can get more compact and readable code:

public static IOrderedEnumerable<TSource> OrderBy<TSource, TKey>(this IQueryable<TSource> source, Expression<Func<TSource, TKey>> keySelector, bool isAscending)
{
    return isAscending
        ? source.OrderBy(keySelector)
        : source.OrderByDescending(keySelector);
}

public static IOrderedQueryable<T_PO> SortedPO(this IQueryable<T_PO> query,  ExtjsGridPagingParameter.SortingParameter sortingParameter)
{
    bool isAscending = sortingParameter.direction == "ASC";

    switch (sortingParameter.property)
    {
        case "po_code":
            return query.OrderBy(item => item.po_code, isAscending);
        case "proj_name":
            return query.OrderBy(item => item.T_PROJECT.proj_name, isAscending);
        case "po_supplier_contact":
            return query.OrderBy(item => item.po_supplier_contact, isAscending);
        case "po_grandtotal":
            return query.OrderBy(item => item.po_grandtotal, isAscending);
        case "po_docdate":
            return query.OrderBy(item => item.po_docdate, isAscending);
        default:
            return null;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Salute you (-/\-) \$\endgroup\$ – Sarawut Positwinyu Jun 24 '13 at 14:39
  • 1
    \$\begingroup\$ I was thinking about using manually constructed Expressions for this, but: 1. It would be messier than this code 2. the special case for proj_name would be hard to do. \$\endgroup\$ – svick Jun 24 '13 at 18:46
  • \$\begingroup\$ @svick it was my first thought as well, but I went this way due to reasons exactly as you described \$\endgroup\$ – almaz Jun 25 '13 at 7:48

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