current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

How will this example affect the using implementation in GetSite?

public abstract class SPSiteBase
{
    protected abstract string Url { get; set; }

    public abstract SPSite GetSite(string url = null);
}

public class SPSiteFactory : SPSiteBase
{
    protected override sealed string Url { get; set; }

    public override SPSite GetSite(string url = null)
    {
        using (SPSite site = new SPSite(string.IsNullOrEmpty(url) ? Url : url))
        {
            return site;
        }
    }

    public SPSiteFactory() { }

    public SPSiteFactory(string url)
    {
        Url = url;
    }
}

I call it like this

SPSiteFactory siteFactory = new SPSiteFactory("http://portalurl/");
SPSite site = siteFactory.GetSite();

I've noticed that the code steps out of the using after I run the siteFactory.GetSite() method but will the site ever be disposed?

share|improve this question
    
using { return } is the same as try { return }—it won't magically handle errors the calling code. Thus using will dispose right away. –  Dan Jan 3 '14 at 20:10

1 Answer 1

up vote 4 down vote accepted

No, you can't use a factory to dispose your objects, your code is creating and immediately disposing the SPSite as soon as it steps out of your method. 

public override SPSite GetSite(string url = null)
{
    return new SPSite(string.IsNullOrEmpty(url) ? Url : url))
}

Have the disposing be handled by the calling code. You can't handle the disposing from a factory method.

If you want to check it out, build your own disposable type, put a breakpoint in the Dispose method and check for yourself.

share|improve this answer
    
That would make returning disposable variables by using using in a factory pattern to an antipattern, that's all I needed, thanks! –  Eric Herlitz Jan 3 '14 at 22:43
1  
@EricHerlitz I wouldn't say it's an anti-pattern. Anti-pattern is something that works, but is bad design. This code simply doesn't work, it returns an object that's already disposed. –  svick Jan 5 '14 at 22:59
    
The instance of site still works but the site in the factory is disposed –  Eric Herlitz Jan 6 '14 at 8:12

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.