HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Generic function With<T> to do a one-liner using block

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericlinerwithblockfunctiononeusing

Problem

I like this. Anyone want to rain on my parade here?

public void LoadDatabasesTest()
{
    string[] args = new[] { "LoadDatabases" };
    ReportsService.Program_Accessor.Main(args);
    With(GetModel(), m => m.Databases.ToList().ForEach(c => Console.WriteLine(c.Name)));
}

void With(T target, Action action) where T : IDisposable
{
    using (target)
    {
        action(target);
    }
}

Solution

What is more clearer here?

This:

using (Model model = GetModel())
{
    foreach (var item in model.Databases)
    {
        Console.WriteLine(item.Name);
    }
}


or this:

With(GetModel(), model =>
    model.Databases.ToList().ForEach(c =>
        Console.WriteLine(c.Name)
    )
)


You've defined some method so you could rearrange some things and add additional levels of indirection to run slower and more inefficient than if you had just written the standard way using the using and foreach blocks. What did you gain here? I'll tell you, nothing.

Any half-decent C# programmer should know what the using and foreach blocks do. There should be no question about it. Seeing your block of code, it's not so clear... we'd have to figure out what you're doing here.

What if someone saw your block of code and wanted to know what is going on? I look at that and see With... "With" what? What the heck is that supposed to mean? If anything, it is only more confusing to people who are familiar with VB.NET's With keyword (which does something completely different). The name isn't very great. You should have given it a more descriptive name. UsingDisposable might have been a better choice or something along those lines.

Don't even get me started on the ToList/ForEach combo... You can never go right doing this instead of using a simple foreach loop. Why would to take a perfectly good collection, copy that collection into another list, just so you can call some method to do stuff with the added overhead of calling another method to do something on each item? The ForEach method has it's place and that's if and only if you start out with a list in the beginning. The overhead of the delegate is more forgivable here since that's what it was designed for. I understand that this particular usage is just a "hack" just to make this loop work, but this should never be the answer. If you must, write an extension at least.

See reason number two of Eric Lippert's "foreach" vs "ForEach". The same can be said here twofold, both for the With() method and your use of ForEach().

What happens if you need to do more than one statement in each of the blocks:

With(GetModel(), model =>
{
    HelloThere();
    model.Databases.ToList().ForEach(c =>
    {
        DoSomething();
        Console.WriteLine(c.Name);
        DoSomethingElse();
    });
    DoMoreStuff();
});


Here come the braces again and even more typing when instead you could have just done:

using (var model = GetModel())
{
    HelloThere();
    foreach (var item in model.Databases)
    {
        DoSomething();
        Console.WriteLine(item.Name);
        DoSomethingElse();
    }
    DoMoreStuff();
}


Save yourself the grief, just use the using and foreach blocks.

Code Snippets

using (Model model = GetModel())
{
    foreach (var item in model.Databases)
    {
        Console.WriteLine(item.Name);
    }
}
With(GetModel(), model =>
    model.Databases.ToList().ForEach(c =>
        Console.WriteLine(c.Name)
    )
)
With(GetModel(), model =>
{
    HelloThere();
    model.Databases.ToList().ForEach(c =>
    {
        DoSomething();
        Console.WriteLine(c.Name);
        DoSomethingElse();
    });
    DoMoreStuff();
});
using (var model = GetModel())
{
    HelloThere();
    foreach (var item in model.Databases)
    {
        DoSomething();
        Console.WriteLine(item.Name);
        DoSomethingElse();
    }
    DoMoreStuff();
}

Context

StackExchange Code Review Q#8845, answer score: 18

Revisions (0)

No revisions yet.