1

In C# you can use a for-loop without declaring the body, what is the benefit of using this in production-code versus using a foreach loop?

Example

My Examples below use reflection to get the property value of an object based on the p_strPropertyPath parameter. The method can also look at the property of a property, for example MyClass.Property.SubProperty.

In my opinion, the foreach loop is more obvious at what it is supposed to do, whereas the former looks cleaner to me.

Body-less For Loop

The example below uses a body-less for-loop and an enumerator:

private string GetValue(MyClass p_objProperty, string p_strPropertyPath) { string[] lstProperties = p_strPropertyPath.Split('.'); IEnumerator objEnumerator = lstProperties.GetEnumerator(); object objValue; for ( objValue = p_objProperty; objValue != null && objEnumerator.MoveNext(); objValue = objValue.GetType().GetProperty(objEnumerator.Current as string).GetValue(objValue) ) ; return Convert.ToString(objValue); } 

foreach loop

The example below uses a foreach loop:

private string GetValue(MyClass p_objProperty, string p_strPropertyPath) { string[] lstProperties = p_strPropertyPath.Split('.'); object objValue = p_objProperty; foreach (string strProperty in lstProperties) { objValue = objValue.GetType().GetProperty(strProperty).GetValue(objValue); if(objValue == null) break; } return Convert.ToString(objValue); } 
12
  • 1
    I don't think these two loops are doing the same thing (hence are not comparable). In particular, the 2nd loop uses an uninitialized variable objValue. Commented Oct 16, 2017 at 16:03
  • 4
    Your first example is just a very clever for loop (a bit too clever). foreach loops are meant to replace for loops in those instances where you don't require the loop variable as an index. The foreach is especially useful here, where it replaces something that's rather unreadable with something that is quite readable. Commented Oct 16, 2017 at 16:03
  • 2
    What's with all the p_xxx variables? Please don't use Hungarian notation in C# code. Use meaningful variables names that clearly convey what the variable does. Even though he deleted the comment, I'm with @RobertHarvey and would want to have stern words with anyone on my team who wrote code like the first example. Always write code so that it is easy to read and thus obvious what it does. Commented Oct 16, 2017 at 16:38
  • 1
    You forgot to dispose the iterator in the first sample. Commented Oct 16, 2017 at 17:20
  • 2
    Good heavens please stop using Hungarian notation in C#. Commented Nov 9, 2017 at 18:16

1 Answer 1

4

Your suggested code is not equivalent if lstProperties is empty, (as ErikEidt points out, also if it is non-empty), if p_objProperty is null, or if .GetValue() returns null. All of those cases are quite relevant because the shown function is using reflection to traverse an object graph.

A for-loop without a body can be an elegant description when searching for a certain value, or when the loop body already happens as a side effect of the loop condition.

I don't think the shown code is a fantastic example of this feature, because both the loop condition and the iterator section have side effects. A classic for-loop is most applicable when all sections are fairly simple. It may be clearer to de-sugar the for-loop into a while loop, mostly because the code is a bit too complex for a single line:

object value = initialValue; while (value != null && enumerator.MoveNext()) { var propertyName = enumerator.Current as string; value = value.GetType().GetProperty(propertyName).GetValue(value); } 

If you were to use a foreach loop, take care to convert the exact loop condition, e.g. by breaking from the loop:

object value = initialValue; foreach (string propertyName in properties) { if (value == null) { break; } value = value.GetType().GetProperty(propertyName).GetValue(value); } 
7
  • thank you for your answer. I hadn't realised I missed the initialisation and null-checking. I updated my question. Do you think the foreach loop is better suited? Commented Oct 16, 2017 at 16:15
  • 1
    @BrownishMonster In the end, that is your call. In C#, I would probably prefer the foreach-loop. This would also be a very good fit for the Linq Aggregate() method which gets rid of the explicit loop: object value = properties.Aggregate(initialValue, (v, property) => v?.GetType().GetProperty(property).GetValue(v)). In other languages, this problem also has an elegant recursive solution. Commented Oct 16, 2017 at 17:51
  • I would pull GetType() out of the loop as well. It is nearly surely returning the same value each time. Commented Oct 17, 2017 at 17:10
  • @FrankHileman That is rather unlikely to be correct, since most objects do not reference other instances of the same type exclusively. OP's GetValue(x, "A.B.C") method is supposed to be the reflection-based equivalent of the expression x.A.B.C, likely as part of a homegrown template engine. Commented Oct 17, 2017 at 17:50
  • I meant to pull it out of the loop, not turn it into a constant. The "value" variable has the same value in the loop, and it's type cannot change. Commented Oct 18, 2017 at 17:34

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.