Is it bad practice to use a using statement on a class level variable?

Question

I have code similar to the following.

class MyController
{
    [ThreadStatic] private DbInterface db;

    public void ImportAllData()
    {
        using (db = new DbInterface())
        {
            var records = PullData();
            PushData(records);
        }
    }

    private DbRecord[] PullData()
    {
        return db.GetFromTableA();
    }

    private void PushData(DbRecord[] records)
    {
        db.InsertIntoTableB(records);
    }
}

The alternative is a lot more cumbersome to maintain.

class MyController
{
    public void ImportAllData()
    {
        using (var db = new DbInterface())
        {
            var records = PullData(db);
            PushData(records, db);
        }
    }

    private DbRecord[] PullData(DbInterface db)
    {
        return db.GetFromTableA();
    }

    private void PushData(DbRecord[] records, DbInterface db)
    {
        db.InsertIntoTableB(records);
    }
}

As far as I can see, my first implementation:

• is thread safe (assuming DbInterface is thread safe),
• prevents any other process from touching the db variable, and
• ensures db will always be disposed, even during an exception.

Is it bad practice to use the using statement on a variable with class scope? Have I missed something?

Answer 1

Personally, I prefer your second option.

The issue with the first design is that your effectively adding unnecessary coupling to the design. Your PullData and PushData methods cannot be used alone - they require that a call to ImportAllData or some other method that will setup and properly cleanup the db variable be called first.

The second option, while slightly more code (though not much), makes the intent very clear for each and every method. Each method knows that it needs to work on an external DbInterface instance passed into it. There is little or no chance of this being misused in the future.