Should I set a license in the constructor or a static constructor?

We are using the Aspose PDF library to work with PDF files. Aspose requires the license to be set once per process. Accordingly, I have created the following common static method that sets the license. Any code that needs Aspose will call into this method.

public class ConfigurationService
{
    private static bool isAsposePdfLicenseSet = false;

    public static void SetAsposePdfLicense()
    {
        if (isAsposePdfLicenseSet)
        {
            return;
        }

        // Set the license... (Moderately expensive - accesses the file system)

        isAsposePdfLicenseSet = true;
    }
}

I need to make sure the license is set before using my class, PdfInspector, but I only need to set it once, so I gave that class a static constructor:

static PdfInspector()
{
    ConfigurationService.SetAsposePdfLicense();
}

However, when I run Code Analysis in Visual Studio, it tells me that static constructors should be avoided. The alternative is to set the license in the constructor:

internal PdfInspector()
{
    ConfigurationService.SetAsposePdfLicense();
}

While this would be called every time the class is instantiated, the SetAsposePdfLicense would short circuit before doing any real logic. Bottom line, which implementation is preferable or is there an alternative that I haven't considered?

This is what made me want to answer:

// Set the license... (Moderately expensive - accesses the file system)

You do not want any constructor, static or not, to do work like this. A constructor that accesses the file system is screaming "I CAN BLOW UP ANYTIME!", and can throw file system -related exceptions. You don't want that in a constructor, even less a static one:


A static constructor is used to initialize any static data, or to perform a particular action that needs to be performed once only. It is called automatically before the first instance is created or any static members are referenced.


http://msdn.microsoft.com/en-us/library/k9x6w0hc.aspx

Making your field static ensures that the type (not the instance) has the field value, and your logic ensures it's set only once in the lifetime of your application. If your application isn't starting any process (requirement is once per process, right?), it can probably work.

I don't like that you're assuming that your application will only ever call this method in the same process though - it's a fair assumption to make, but since the requirements say once per process, I think your code should account for System.Diagnostics.Process.GetCurrentProcess().Id, and call the SetAsposePdfLicense() method if the method wasn't called in the process your code is running in.

I'm thinking of a factory living in a Singleton scope (or just as a plain Singleton), that would maintain an ICollection (a HashSet would be ideal for this) where each item is a process ID for which SetAsposePdfLicense was called.

public class PdfInspectorFactory
{
    private readonly ICollection _processes = new HashSet();

    private void SetLicenseForCurrentProcess()
    {
        var currentProcessId = System.Diagnostics.Process.GetCurrentProcess().Id;
        if (_processes.Contains(currentProcessId))
        {
            return;
        }

        // set the license...

        _processes.Add(currentProcessId);
    }

    public PdfInspector Create()
    {
        SetLicenseForCurrentProcess();
        var inspector = new PdfInspector();

        // do whatever you need to do with your inspector before returning it

        return inspector;
    }
}

Then your client code can use this PdfInspectorFactory and call its Create method to get a PdfInspector without having to care about whether or not the license is set.

The client code will not call the factory method in its constructor, rather only when it needs it.

public class SomeClass
{
    private readonly PdfInspectorFactory _factory;

    public SomeClass(PdfInspectorFactory factory)
    {
        _factory = factory;
    }

    public void DoSomething()
    {
        try
        {
            var inspector = _factory.Create();
            // here you go, now do what you need to do with your inspector...

        }
        catch(Exception exception)
        {
            // todo: log or otherwise report exception
            throw;
        }
    }
}

The factory instance should only exist as a Singleton instance - if you're using an IoC container that is very easy to accomplish. Otherwise you need to implement the factory class itself, as a Singleton.

StackExchange Code Review Q#54506, answer score: 6