Ordering a specific property - code improvement

Question

I have a way like this:

public void DoSomething( .... , bool orderByX) { if(orderByX) { foreach( ... OrderBy(x => x.Location.X)) { ... } } else { foreach( ... OrderBy(x => x.Location.Y) { ... } } } 

I would like to avoid if to create less repetitive code (i.e. only one foreach). Is it possible?

Thanks.

Answer 1

The best approach is to pass the criteria by which you can order. You can use the following code as a motivation:

 public void DoSomething<T>( .... , Func<Point, T> orderbySelector) { foreach( ... OrderBy(p => orderbySelector(p.Location))) { ... } } 

Now you can:

 DoSomething(mySequence, point => point.X) 

or

 DoSomething(mySequence, point => point.Y) 

Note. You can generalize the selector as much as you want (for example, pass the holder or Location , not Point ).

In addition, passing bool as order criteria makes the code less readable. For example, I don’t know what this method does, just looking at it, call DoSomething(list, false) , and I need to see the signature of the method to find out what the semantics of false . It would be much better to use the named parameters DoSomething(list, orderByX : false) (available from C # 4.0 ), but if I don't order X , how do I know that I will then be ordered to Y ? . It also limits the calling code to only two sorting criteria (you would not want to add another sorting flag, would you?)

So, you need to open your intention by making DoSomething name indicating that you are actually ordering your processing. For example TraverseNodesOrderedBy(nodes, point => point.X)

Answer 2

Check orderByX in lambda expression for OrderBy

 public void DoSomething( .... , bool orderByX) { foreach( ... OrderBy(x => orderByX ? x.Location.X : x.Location.Y)) { ... } } 

Answer 3

LINQ queries are composite, which means you can create them before they execute:

 public void DoSomething( .... , bool orderByX) { var query = ... ; if (orderByX) query = ... .OrderBy(x => x.Location.X); else query = ... .OrderBy(x => x.Location.Y); foreach(var x in query) // deferred execution { ... } } 

Another option, among other viable answers.