Ytdyklly

There is a service class (ProposalPdfService) in one of our assemblies that has implementation logic specific to that app's assembly. Previously (and, I believe, erroneously) I treated it as a quasi-singleton by making it and all its members static, but I refactored it to use the singleton pattern described here using the Lazy<T> type.

Upon needing to share a generic subset of this service's functionality with another assembly, I decided to move that functionality into a base class (PdfServiceBase) in a shared assembly used by both apps.

This second app can use the base service as-is — there are currently no plans to add any implementation-specific logic for this app — while the main app uses ProposalPdfService, which inherits from PdfServiceBase. Both classes are singletons.

public class PdfServiceBase

{

    private static readonly Lazy<PdfServiceBase> lazy = new Lazy<PdfServiceBase>(() => new PdfServiceBase());



    public static PdfServiceBase Instance { get => lazy.Value; }



    protected PdfServiceBase() { }



    public PdfDocument RetrievePdfFile(string relativeFilePath, string relativeFileName)

    {

        PdfDocument pdf = null;

        string directoryWithoutLastBackSlash = FilePathManager.GetDocumentsFilePath("PDFs");

        string filePath = directoryWithoutLastBackSlash + @"" + relativeFilePath;

        string fileName = filePath + @"" + relativeFileName + ".pdf";



        // Check if file exists

        if (System.IO.File.Exists(fileName))

            pdf = PdfDocument.FromFile(fileName);



        return pdf;

    }

}

public sealed class ProposalPdfService : PdfServiceBase

{

    private static readonly Lazy<ProposalPdfService> lazy = new Lazy<ProposalPdfService>(() => new ProposalPdfService());



    public static new ProposalPdfService Instance { get => lazy.Value; }



    private ProposalPdfService() { }



    // app-specific service members listed below...

    public string Foo { get; } = "foo";

}

I specifically chose inheritance over containment because, in my opinion, it would be very clumsy to have to remember to use the base class service just to use the RetrievePdfFile method. Containment would also require having an instance of ProposalPdfService and PdfServiceBase. Being able to invoke ProposalPdfService.RetrievePdfFile makes good sense to me.

The only thing that feels "weird" about this implementation to me is that, due to both classes being able to be instantiated as independent singletons, they both have Instance members, meaning that the derived property has to shadow the base property using the new keyword. And I know shadowing is one of those things people typically associate with poor design.

I'm generally happy with this implementation, but as this is my first time implementing a "real" singleton and, due to the inheritance, it's not so straight forward, I wanted to get some feedback on it. Do you see any problems with this approach? Or, if it's the right approach, are there any improvements I could make?

I believe a requirement of a “true” singleton is that it’s supposed to be sealed and can’t be inherited. The very article you linked states as much. This is why things feel a bit off to you, you’re only respecting the pattern for one of the classes.

Consider having PdfServiceBase be an abstract class that implements RetrievePdfFile() and any other common code like that, then implement ProposalPdfService and something like just PdfService as singletons each implementing the pattern properly and separately, without “new”.