More fluent C# / .NET

Question

A co-worker of mine came up with this and I wonder what others think? Personally, I find it interesting but wonder if it is too big a departure? Code examples below. Extension methods at the bottom.

General thoughts please. Other extension methods that could be added?

var ddl = Page.FindControl("LocationDropDownList") as DropDownList;

ddl.Visible = true;
ddl.SelectedValue = "123";

if(isAdmin)
    ddl  .SelectedValue = "111";

Becomes:

Page.FindControl("LocationDropDownList")
    .CastAs<DropDownList>()
    .With(d => d.Visible = true)
    .With(d => d.SelectedValue = "123")
    .WithIf(isAdmin, d => d.Items.Add(new ListItem("Admin", "1")));

Or:

 Page.FindControl("LocationDropDownList")
       .CastAs<DropDownList>()
       .With(d =>
       {
           d.Visible = true;
           d.SelectedValue = "123";
       })
       .WithIf(isAdmin, d => d.SelectedValue = "111");

Extension methods:

public static TResult CastAs<TResult>(this object obj) where TResult : class
{
    return obj as TResult;
}

public static T With<T>(this T t, Action<T> action)
{
    if (action == null)
        throw new ArgumentNullException("action");

    action(t);

    return t;
}

public static T WithIf<T>(this T t, bool condition, Action<T> action)
{
    if (action == null)
        throw new ArgumentNullException("action");

    if (condition)
        action(t);

    return t;
}

Answer 1

The original is more readable.

The simplest API change would be to make the object returned by FindControl() a Builder-esque thing (where all the set methods return 'this'):

Page.FindControl("LocationDropDownList")
    .setVisible(true)
    .setSelectedValue(isAdmin ? "111" : "123");

Answer 2

This is a perfect learning case on how to make something more complicated than it needs to be.

The first version is clear and requires no extra knowledge beyond normal language contructs.

Answer 3

Good rule of thumb:

If your first impression of your code is "This is clever" - it's probably not a good idea.

Good code should be simple, readable, and only "clever" if absolutely necessary.

Answer 4

I am more comfortable with the first version. It takes less time to read and understand. I agree that the extension methods are also fine if you are familiar with it and also familiar with the With method, but what’s the benefit of it in this case?

Answer 5

Amongst my rules of thumb for writing clear code is: put all side effects in statements; non-statement expressions should have no side effects.

Your first version of the program clearly follows this rule. The second version clearly violates it.

An additional thought: if I were to read code like the code you've displayed, I would naturally assume that the purpose of the code was to build up a lazily-evaluated structure which represented those operations -- this is exactly why query comprehensions in C# 3 are built in this way. The result of the query expression is an object representing the deferred application of the query.

If your intention is to capture the notion of "execute these side effects in a deferred manner at a later moment of my choosing", then this is a sensible approach. Essentially what you're building up is a side-effecting monad. If your intention is merely to provide a different syntax for the eagerly executed code, then this is just confusing, verbose and unnecessary.

Answer 6

Regarding a "Fluent Interface" C# already has a great syntax for initializers which is (IMHO) better that trying to use the fluent style. Of course, in your example you are not initializing an new object, you are changing an existing one. My whole expertise with Fluent interfaces comes from a 30 second scan of wikipedia, but I think that JeeBee's answer is more in the spirit of Fluent programming, though I might change things slightly:

Page.FindDropDownList("LocationDropDownList")    
  .setVisible(true)    
  .setAdminSelectedValue("111")
  .setSelectedValue("123")

One could argue that this is more readable, especially for a language without Properties, but I still think it doesn't make sense in C#.