Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
2

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;
    }
share|improve this question

1 Answer

up vote 4 down vote accepted

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;
    }
}
share|improve this answer
 
Salute you (-/\-) –  Sarawut Positwinyu Jun 24 at 14:39
 
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. –  svick Jun 24 at 18:46
 
@svick it was my first thought as well, but I went this way due to reasons exactly as you described –  almaz Jun 25 at 7:48

Your Answer

 
discard

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.