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);
}
  • 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. – Erik Eidt Oct 16 '17 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. – Robert Harvey Oct 16 '17 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. – David Arno Oct 16 '17 at 16:38
  • 1
    You forgot to dispose the iterator in the first sample. – CodesInChaos Oct 16 '17 at 17:20
  • 2
    Good heavens please stop using Hungarian notation in C#. – Eric Lippert Nov 9 '17 at 18:16
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);
}
  • 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? – Brownish Monster Oct 16 '17 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. – amon Oct 16 '17 at 17:51
  • I would pull GetType() out of the loop as well. It is nearly surely returning the same value each time. – Frank Hileman Oct 17 '17 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. – amon Oct 17 '17 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. – Frank Hileman Oct 18 '17 at 17:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.