Conditional Using

April 9th, 2009

Today I found myself writing the following code:

private void DoSomething(String userName)
{
    using(var userEntry = GetUserEntryFor(userName))
        if(CouldBeFound(userEntry))
    {
        // Do something with userEntry                
    }
} 

private DirectoryEntry GetUserEntryFor(String userName)
{
    // ...        
} 

private static Boolean CouldBeFound(DirectoryEntry entry)
{
     return null != entry;
}

Notice the combination of the if statement with the using statement. I thought this was kind of neat. What do you think?

Is the style of this code bonafide?

Jan Van Ryswyck Esoterica

  1. April 9th, 2009 at 14:59 | #1

    Neat yes,

    But why not just do a null check? is there ever a possibility that CouldBeFound is going to do anything more then checking to see if entry is null?

    your basically turning one line of code into 3, and kinda makes it harder to read.

    Now if you where going to be doing something more then just doing a null check, like doing some form of validation then yes it would be a good coding standard. In this case I think your over complicating it. that’s just my 2 cents.

  2. Chris M
    April 9th, 2009 at 15:42 | #2

    Bonafide? No.

    var userEntry = GetUserEntryFor(userName);
    if(userEntry == null)
    return;

    using (userEntry)
    {
    // Do something with userEntry
    }

  3. April 9th, 2009 at 15:51 | #3

    I would rather see an extension method on the userEntry so I could do this:

    if(userEntry.IsNotNull())

    or something like that.

  4. April 9th, 2009 at 16:52 | #4

    I like the David’s Idea of doing a null check extender, something like this

    public static bool IsNotNull(this T myObject)
    {
    return myObject != null;
    }

    vs the CouldBeFound method, then you have a not null for everything, and it’s nice and readable

  5. April 10th, 2009 at 04:00 | #5

    Sry, guys, no extension methods. I’m targeting .NET 2.0 here.

    @Chris: The code you showed is what I had at first. But I had several DirectoryEntry objects that way, so I found this syntax to be more compact but still readable.

  6. April 10th, 2009 at 08:11 | #6

    I didn’t know you could use ‘if’ with ‘using’ How does it evaluate? The instanace get checked first before going into the code block? so if is allowed, what else can you do with using? I never seen this code before. Thanks for the tip.

  7. Robert
    April 10th, 2009 at 08:38 | #7

    I’m not a fan of being clever for the sake of being clever. I prefer the much more explicit and boring:

    using(var userEntry = GetUserEntryFor(userName))
    {
    if(userEntry != null)
    {
    // Do something with userEntry
    }
    }

    Everyone (should) knows what the above means. From the comments what you did confused some people.

    If at some point the test for userEntry becomes more complex maybe a method is warrented. But the test should always be inside the explicitly braced using block.

  8. April 10th, 2009 at 09:23 | #8

    I’m starting to get more and more annoyed by the braces masochism that the C# compiler enforces. This is probably why I ended up with this code. The code inside the using is only executed when the if statements evaluates to true. Maybe its time for me to evaluate some other languages?

  9. April 10th, 2009 at 18:11 | #9

    Yeah but the braces masochism came from C, where implicit braces were built into the language – and quickly became an awesome source of the best sorts of hard to find bugs. It became an anti-pattern, for a good reason.

    True, we have things better now, with IDEs and tools that will help catch problems. But still, to me, leaving off the outer {}s for the using block strikes me a just being clever to prove a point. Who cares about two extra lines of brace, vs. the risk of subtle bugs and confusing the cow orkers who follow you?

Comments are closed.