debugcsharpModerate
Imposing a sort order on a query
Viewed 0 times
sortqueryorderimposing
Problem
I've always been told "never use goto, there's always a better way" and for the longest time I just accepted it. Lately though, I've been running into such scenarios in which I have to repeat this bit of code every time I return early.
I personally hate repeating myself, even the littlest bit of code. So I ask, Is this use of goto really that bad? It prevents me from having to write that bit of code 3 times and sure, theoretically I could write some guard clauses (that could be combined into a single guard clause) that prevents the need for the try/catch and therefore having a single place for that bit of code, but would that actually be more readable?
I personally hate repeating myself, even the littlest bit of code. So I ask, Is this use of goto really that bad? It prevents me from having to write that bit of code 3 times and sure, theoretically I could write some guard clauses (that could be combined into a single guard clause) that prevents the need for the try/catch and therefore having a single place for that bit of code, but would that actually be more readable?
public static IQueryable Sort(this IQueryable query, String field, String direction, Expression> defaultSort = null)
{
if (String.IsNullOrWhiteSpace(field)) { goto FAILED; }
try
{
// Dynamic LINQ - Field: "SomeProperty.OptionalProperty", Direction: "Desc" or "Asc"
query = query.OrderBy(String.Format("{0} {1}", field, direction));
}
// NOT FOUND: Field wasn't found just return the original query.
catch (NullReferenceException)
{
goto FAILED;
}
// NOT FOUND: Field wasn't found just return the original query.
catch (ParseException)
{
goto FAILED;
}
return query;
// SOMETHING FAILED: Return the original query with the optionally supplied default sort.
FAILED:
if (defaultSort != null)
{
query = query.OrderBy(defaultSort);
}
return query;
}Solution
I'm not a member of the “
goto is evil and must not be used under any circumstances” camp. Especially in performance-critical low-level routines, it can be useful from time to time. However, most of the time, the temptation to use goto actually stems from missing a better opportunity to structure the code. In your example, I'd simply re-structure it like this, which I find is not only more readable but also much shorter.public static IQueryable
Sort(this IQueryable query,
String field,
String direction,
Expression> defaultSort = null)
{
if (!String.IsNullOrWhiteSpace(field)) {
try {
return query.OrderBy(String.Format("{0} {1}", field, direction));
} catch (NullReferenceException) {
// NOT FOUND: Field wasn't found just return the original query.
} catch (ParseException) {
// NOT FOUND: Field wasn't found just return the original query.
}
}
if (defaultSort != null) {
query = query.OrderBy(defaultSort);
}
return query;
}Code Snippets
public static IQueryable<T>
Sort<T>(this IQueryable<T> query,
String field,
String direction,
Expression<Func<T, Int32?>> defaultSort = null)
{
if (!String.IsNullOrWhiteSpace(field)) {
try {
return query.OrderBy(String.Format("{0} {1}", field, direction));
} catch (NullReferenceException) {
// NOT FOUND: Field wasn't found just return the original query.
} catch (ParseException) {
// NOT FOUND: Field wasn't found just return the original query.
}
}
if (defaultSort != null) {
query = query.OrderBy(defaultSort);
}
return query;
}Context
StackExchange Code Review Q#95265, answer score: 19
Revisions (0)
No revisions yet.