How to prevent Factory Method pattern causing warning about virtual member call in constructor?

Question

On www.dofactory.com I found a real world example of the Factory Pattern. But the code generates a warning in ReSharper about a virtual member call in the constructor.

The code causing the warning is the following:

abstract class Document
{
    private List<Page> _pages = new List<Page>();

    // Constructor calls abstract Factory method
    public Document()
    {
        this.CreatePages(); // <= this line is causing the warning
    }

    public List<Page> Pages
    {
        get { return _pages; }
    }

    // Factory Method
    public abstract void CreatePages();
}

class Resume : Document
{
    // Factory Method implementation
    public override void CreatePages()
    {
        Pages.Add(new SkillsPage());
        Pages.Add(new EducationPage());
        Pages.Add(new ExperiencePage());
    }
}

In the consuming code, you can then simply use:

Document document = new Resume();

I do understand why it's a bad idea to call a virtual member in the constructor (as explained here).

My question is how you can refactor this in order to still use the factory pattern, but without the virtual member call in the constructor.

If I'd just remove the call to CreatePages from the constructor, the consumer would have to explicitly call the CreatePages method:

Document document = new Resume();
document.CreatePages();

I much more prefer the situation where creating a new Resume is all that's needed to actually create a Resume containing pages.

Answer 1

One way to refactor this would be passing the pages upfront, and passing them to a protected constructor:

public abstract class Document {
    protected Document(IEnumerable<Page> pages) {
        // If it's OK to add to _pages, do not use AsReadOnly
        _pages = pages.ToList().AsReadOnly();
    }
    // ...
}

public class Resume : Document {
    public Resume() : base(CreatePages()) {
    }
    private static IEnumerable<Page> CreatePages() {
        return new Page[] {
            new SkillsPage(),
            new EducationPage(),
            new ExperiencePage()
        };
    }
}

P.S. I am not sure what this has to do with the factory method. Your post illustrates the Template Method Pattern.

Answer 2

What about this? It uses lazy initialization where the pages are created only when needed (instead of creating them in the constructor)

Also, note the factory method visibility is changed to protected to hide it from public usage.

abstract class Document{
    protected List<Page> _pages = new List<Page>();

    // Constructor calls abstract Factory method
    public Document(){}

    public List<Page> Pages
    {
        get { CreatePages(); return _pages; }
    }

    // Factory Method
    protected abstract void CreatePages();
}

class Resume : Document{
    // Factory Method implementation
    protected override void CreatePages(){
       if(pages.Count == 0 ){
        _pages .Add(new SkillsPage());
        _pages .Add(new EducationPage());
        _pages .Add(new ExperiencePage());
       }
    }
}

EDIT Suggestion: I personally do not like having that global _pages variable as it might get you into trouble if shared among many methods and threads. I would rather go for the factory method pattern as described in the GoF book. Here is my suggestion:

abstract class Document{
    public IEnumerable<Page> Pages{
        get { return CreatePages();}
    }

    // Factory Method
    protected abstract IEnumerable<Page> CreatePages();
}

class Resume : Document{
    // Factory Method implementation
    protected override IEnumerable<Page> CreatePages(){
         List<Page> _pages = new List<Page>();
        _pages .Add(new SkillsPage());
        _pages .Add(new EducationPage());
        _pages .Add(new ExperiencePage());
        return _pages;
       }
    }
}

Answer 3

My question is how you can refactor this in order to still use the factory pattern, but without the virtual member call in the constructor.

According to the definition

In class-based programming, the factory method pattern is a creational pattern which uses factory methods to deal with the problem of creating objects without specifying the exact class of object that will be created.

factory method is not intended to be used from the constructor, since the exact class of object that will be created is known at construction time. For example,

1. if Document must create all Pages at construction time, then it can do it without factory method since it knows exactly all the Pages to be created

2. but if Document must create Pages later after construction and maybe multiple times, then factory method is useful

I much more prefer the situation where creating a new Resume is all that's needed to actually create a Resume containing pages.

So, if all Pages are to be created at construction time, this concrete example can be rewritten without use of factory method pattern:

public class Document
    private readonly PageList as IList(of IPage)

    public readonly property Pages as IEnumerable(of IPage)
        get
            return PageList
        end get
    end property

    public sub new()
        Me.PageList = new List(of IPage)
    end sub

    protected sub Add(paramarray Pages() as IPage)
        Me.Pages.AddRange(Pages)
    end sub
end public

public class Resume
    inherits Document

    public sub new()
        mybase.add(new SkillsPage, new EducationPage, new ExperiencePage)
    end sub
end class

1. Method Add is allowed to be used from Resume constructor, since Document object is fully constructed and ready to be used.

2. Method Add is protected, so pages can only be be added from Document or derived classes.