The complex way is not really the right way…

Posted on September 28, 2009 by cristian

Today I was visiting a client, and I just stop and watch something that really caught my attention: 

foreach (item in myList) {
    if (item.Length > 0) {
        SaveItem(item);
    }
}
// more ugly code...

Well, the code was really different (an uglier one) but you got the idea…. What I suggested to my client was a more concise solutio, create an extension method: 

public static class IEnumerableExt {
    public static IEnumerable<T> ForEach(this IEnumerable<T> items, Action<T> action) {
        foreach(var item in items) {
            action(e);
        }
    }
}

The second part is the easy one… 

myList
    .Where((item) => item.Length > 0)
    .ForEach((item) => SaveItem(Item));

I don’t know you, but this way looks a lot clearer to me…

Lessson learned: If your code looks complex, please, take a time and look how to make it more clearer.

This entry was posted in development and tagged , , . Bookmark the permalink.
  • Carlos Peix

    Hi Cristian,

    foreach (item in myList)
    if (item.Length > 0)
    SaveItem(item);

    Does this code look complex? really?

    Why? it uses usual statements and certainly is understood by a much broader audience than this one:

    myList
    .Where((item) => item.Length > 0)
    .ForEach((item) => SaveItem(Item));

    Not to mention the extension method definition and (needed) reference

    Thanks

  • Carlos Peix

    Hi Cristian,

    foreach (item in myList)
    if (item.Length > 0)
    SaveItem(item);

    Does this code look complex? really?

    Why? it uses usual statements and certainly is understood by a much broader audience than this one:

    myList
    .Where((item) => item.Length > 0)
    .ForEach((item) => SaveItem(Item));

    Not to mention the extension method definition and (needed) reference

    Thanks

    • http://www.cprieto.com cprieto

      Well, the code I was reviewed was NOT as simple as your current statement… With something as simple as your statement I will prefer to let it be as is, without *any* modification (but add curly braces).

      • Carlos Peix

        I understand, but with or without curly braces, I still prefer the original.

        I realise, as well, that the actual code you reviewed should had been uglier.

        Thanks

  • http://www.cprieto.com cprieto

    Well, the code I was reviewed was NOT as simple as your current statement… With something as simple as your statement I will prefer to let it be as is, without *any* modification (but add curly braces).

  • Carlos Peix

    I understand, but with or without curly braces, I still prefer the original.

    I realise, as well, that the actual code you reviewed should had been uglier.

    Thanks

  • Carlos Peix

    Hi Cristian,

    foreach (item in myList)
    if (item.Length > 0)
    SaveItem(item);

    Does this code look complex? really?

    Why? it uses usual statements and certainly is understood by a much broader audience than this one:

    myList
    .Where((item) => item.Length > 0)
    .ForEach((item) => SaveItem(Item));

    Not to mention the extension method definition and (needed) reference

    Thanks

  • http://www.cprieto.com cprieto

    Well, the code I was reviewed was NOT as simple as your current statement… With something as simple as your statement I will prefer to let it be as is, without *any* modification (but add curly braces).

  • Carlos Peix

    I understand, but with or without curly braces, I still prefer the original.

    I realise, as well, that the actual code you reviewed should had been uglier.

    Thanks