current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Stack Overflow is a community of 4.7 million programmers, just like you, helping each other.

Join them; it only takes a minute:

Sign up
Join the Stack Overflow community to:
  1. Ask programming questions
  2. Answer and help your peers
  3. Get recognized for your expertise
up vote 2 down vote favorite

I have created a PhoneBook style application; on my phonebook object I have a local member _site which is used as a filter, since there are approximately 1000 phone numbers, split across 12 sites within my organisation. Only one site will be retrieved at a time using this method.

This was my original method. The GUI has several methods for reordering the data, so I left it as an IQueryable because I would like to defer SQL to allow for filtering to be done on the SQL server rather than on the client PC.

Works

public IQueryable<PhoneNumber> GetPhoneDirectory()
{
    PhoneBookDataContext db = new PhoneBookDataContext())
    return db.PhoneNumbers.Where(d => d.Site == _site);
}

However, I am also trying to keep to 'best practise' in terms of using statements.

Doesn't Work

public IQueryable<PhoneNumber> GetPhoneDirectory()
{
    using (PhoneBookDataContext db = new PhoneBookDataContext())
    {
        return db.PhoneNumbers.Where(d => d.Site == _site);
    }
}

Now as pointed out by @justanotheruseryoumay, this will cause an exception because the datacontext is disposed by the time the objects are accessed.

I guess what I am asking is, how can I make sure my data context is disposed nicely, when I cannot use a 'using' statement and don't strictly know when the context is done with.

share|improve this question
    
Though I can't be sure, I believe the problem is that the using statement wouldn't be able to Dispose the object until it fell out of scope -which wouldn't happen until the query were to execute. – Mike Perrenoud Jun 10 '13 at 13:00
    
Someone correct me if I'm wrong here, but wont any local variables get marked for disposal the second a method returns? I assumed the two snippets above were essentially equivalent. – Gray Jun 10 '13 at 13:01
    
Hi, I've edited my question, since I didn't clearly mention that the second throws an exception. – KingCronus Jun 10 '13 at 13:01
    
You should let @justanotheruseryoumayknow as it appears their answer is correct. – dash Jun 10 '13 at 13:08
    
They deleted it, can't figure out how to contact them. – KingCronus Jun 10 '13 at 13:09
up vote 4 down vote accepted

If you want to return IQueryable you can make your class that contains the GetPhoneDirectory disposable, make the PhoneBookDataContext a field, and dispose it in your dispose method.

You will then put the onus on the caller to dispose his instance of your class.

E.g.

public class MyClass : IDisposable
{
    PhoneBookDataContext db;

    public MyClass()
    {
        db = new PhoneBookDataContext();
    }

    public IQueryable<PhoneNumber> GetPhoneDirectory()
    {
        return db.PhoneNumbers.Where(d => d.Site == _site);
    }

    public void Dispose()
    {
        if (db != null)
        {
            db.Dispose();
            db = null;
        }
    }
}

// Caller
using(var myClass = new MyClass())
{
    var queryable = myClass.GetPhoneDirectory();
    ...
}
share|improve this answer
    
Ah I like the look of this! My only question is what happens if somebody else inserts a record into PhoneNumbers between the time that somebody creates "MyClass" and then calls the method? Presumably I always get a fresh call to SQL? – KingCronus Jun 10 '13 at 13:12
    
@KingCronus You do. – Servy Jun 10 '13 at 13:56
up vote 0 down vote

The execution of the query will still be deferred and the PhoneBookDataContext will still be properly Disposed because using is interpreted by the compile as a try/finally. When you actually execute the query it will result in a runtime error because the PhoneBookDataContext no longer exists. I would suggest doing a .ToList() on your query and returning it that way. If you want to change the order after you return it then you can still do LINQ on it as you please.

EDIT: Another thing you could do is to create the using with the PhoneBookDataContext in the calling method and pass in the context. The context is really going to be used in that method anyway and you can keep it around as long as you need it and stick with the good using format.

share|improve this answer
up vote 0 down vote

Yes; It is bad design because your IQueryable<PhoneNumber> will be evaluated only when you call a method that cause it to be evaluated, like ToList() or when you iterate it with foreach.

in your code you are returning a IQueryable<PhoneNumber> which is not evaluated yet, and before the caller get any chance to execute it, it's internals that has the responsibility for yielding the records to you (db); is already disposed.

Just as a suggestion:

public IEnumerable<PhoneNumber> GetPhoneDirectory()
{
    using (PhoneBookDataContext db = new PhoneBookDataContext())
    {
        return db.PhoneNumbers.Where(d => d.Site == _site).ToList();
    }
}

Or relocate the db object to somewhere else in your design (Unit Of Work and Repository are nice patterns to get a look at IMHO).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.