patterncsharpMinor
Ordering filtered results
Viewed 0 times
resultsorderingfiltered
Problem
I have the following method which under code review gives me a CA1502 avoid excessive complexity error (although if I suppress the message it will build fine):
Explanation of variables:
Address object class:
Is there a way to make this code less
private void OrderResults()
{
switch (_orderColumn)
{
case 0:
default:
_fitleredResults = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase) ? _fitleredResults.OrderByDescending(x => x.Street) : _fitleredResults.OrderBy(x => x.Street);
break;
case 1:
_fitleredResults = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase) ? _fitleredResults.OrderByDescending(x => x.Street) : _fitleredResults.OrderBy(x => x.Street);
break;
case 2:
_fitleredResults = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase) ? _fitleredResults.OrderByDescending(x => x.City) : _fitleredResults.OrderBy(x => x.City);
break;
case 3:
_fitleredResults = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase) ? _fitleredResults.OrderByDescending(x => x.Country) : _fitleredResults.OrderBy(x => x.Country);
break;
case 4:
_fitleredResults = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase) ? _fitleredResults.OrderByDescending(x => x.RecID) : _fitleredResults.OrderBy(x => x.RecID);
break;
}
}Explanation of variables:
_filteredResultsis anIEnumerable;
_orderColumnis an int passed in by jQueryDataTables and doesn't match the property indexes (which is why I was mapping it using a switch)
Address object class:
public class AddressObject
{
public string City { get; set; }
public string Country { get; set; }
public string CountryRegionID { get; set; }
public string Postcode { get; set; }
public long RecID { get; set; }
public string Street { get; set; }
}Is there a way to make this code less
Solution
Firstly, your
I normally see
You're repeating this code a lot:
So far:
As Heslsacher says, you could further improve by using named constants for the indexes.
I did write some additional stuff about using a method to get a Func but then I realised that
Edit
For posterity, I'll add in what I had written:
case 1 case is identical to your case 0,default case (order by street). Is that intentional? It looks like a mistake to me because you've cascaded 0 and default but not 1. If it isn't a mistake, and they do all have the same behaviour you should cascade them all or let them all be picked up by the default case.I normally see
default as the last option in a switch statement, I feel like that might be a convention but don't have anything to back it up.You're repeating this code a lot:
_orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase) put it in a variable (which will make the code more readable.So far:
private void OrderResults()
{
var isDescending = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase);
switch (_orderColumn)
{
case 2:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.City) : _fitleredResults.OrderBy(x => x.City);
break;
case 3:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.Country) : _fitleredResults.OrderBy(x => x.Country);
break;
case 4:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.RecID) : _fitleredResults.OrderBy(x => x.RecID);
break;
default:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.Street) : _fitleredResults.OrderBy(x => x.Street);
break;
}
}As Heslsacher says, you could further improve by using named constants for the indexes.
I did write some additional stuff about using a method to get a Func but then I realised that
RectId is a long so it all went out the window.Edit
For posterity, I'll add in what I had written:
private void OrderResults()
{
var isDescending = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase);
var sortProperty = GetSortProperty();
if (isDescending)
{
_filteredResults = _filteredResults.OrderByDescending(sortProperty);
}
else
{
_filteredResults = _filteredResults.OrderBy(sortProperty);
}
}
private Func GetSortProperty()
{
switch(_orderColumn)
{
case 2:
return x => x.City;
case 3:
return x => x.Country;
case 4:
return x => x.RectId;
default:
return x => x.Street;
}
}Code Snippets
private void OrderResults()
{
var isDescending = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase);
switch (_orderColumn)
{
case 2:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.City) : _fitleredResults.OrderBy(x => x.City);
break;
case 3:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.Country) : _fitleredResults.OrderBy(x => x.Country);
break;
case 4:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.RecID) : _fitleredResults.OrderBy(x => x.RecID);
break;
default:
_fitleredResults = isDescending ? _fitleredResults.OrderByDescending(x => x.Street) : _fitleredResults.OrderBy(x => x.Street);
break;
}
}private void OrderResults()
{
var isDescending = _orderDirection.Equals("desc", StringComparison.InvariantCultureIgnoreCase);
var sortProperty = GetSortProperty();
if (isDescending)
{
_filteredResults = _filteredResults.OrderByDescending(sortProperty);
}
else
{
_filteredResults = _filteredResults.OrderBy(sortProperty);
}
}
private Func<AddressObject, object> GetSortProperty()
{
switch(_orderColumn)
{
case 2:
return x => x.City;
case 3:
return x => x.Country;
case 4:
return x => x.RectId;
default:
return x => x.Street;
}
}Context
StackExchange Code Review Q#93746, answer score: 5
Revisions (0)
No revisions yet.