1

I stumbled on following problem, and I'm curious if it could be done better.

A while ago I wrote a factory class that looked something like this:

    public class Foo
    {
        private IDbContext _dbContext;
        
        public Foo(IDbContext dbContext)
        {
            _dbContext = dbContext;
        }
    
        public MyObject CreateMyObject(string propA)
        {
            return new MyObject{PropFromDb = _dbContext.propFromDb, PropA = propA};
        }
    }

It was created at the top of my call stack, then passed down, to finally call CreateMyObject somewhere at the bottom. It worked fine, because there were not that many layers, and created object was pretty simple. Some time passed, few more layers arised with few new properties that needed to be passed to my object.

So I turned my class into a builder:

    public class Foo
    {
        private IDbContext _dbContext;
        private string _propA;
        private string _propB;
        
        public Foo AddDbContext(IDbContext dbContext)
        {
            _dbContext = dbContext;
            return this;
        }
    
        public Foo AddPropA(string propA)
        {
            _propA = propA;
            return this;
        } 

    ...    

        public MyObject Build()
        {
            return new MyObject{PropFromDb = _dbContext.propFromDb, PropA = propA, PropB = propB};
        }
    }

It works fine, but there is the following issue that I have with this approach:

App
 - ServiceA
  --SubServiceA
  --SubServiceB
 - ServiceB

If I create my builder at the top, then pass it down, decorating it along the way with properties, then call Build in the SubServiceA, every other service that shares builder instance (i.e. ServiceB) will have it filled with properties from different context. As long as I remember to redecorate it again this is fine, but this does not sound good, as someone creating new method must know to do this as well.

I know that I can create Reset method, that would clear builder internal state, determining when to call it seems problematic, sometimes I would like to clear only one property, keep the rest and pass it down to another method.

I could also pass down new instance of Foo each time I need it, but params such as IDbContext in this case wont change within my app, and could be shared by all layers.

I came up with the idea of copying internal state to new instance each time I decorate my builder, and then pass down new instance, which looks something like this:

public class Foo
{
    private IDbContext _dbContext;
    private string _propA;
    private string _propB;

    public Foo()
    {
    }

    private Foo(IDbContext dbContext, string propA, string propB)
    {
        _dbContext = dbContext;
        _propA = propA;
        _propB = propB;
    }

    public Foo AddDbContext(IDbContext dbContext)
    {
        _dbContext = dbContext;
        return new Foo(_dbContext, _propA, _propB);
    }

    public Foo AddPropA(string propA)
    {
        _propA = propA;
        return new Foo(_dbContext, _propA, _propB);
    }

...

    public MyObject Build()
    {
...
    }
}

Are those best-practice approaches, or am I completely missing something, and this could be done differently?

1
  • 1
    If your services clearly demarcate "reuse boundaries" (for the lack of a better phrase), can you maybe simply pass a different instance of your builder to each service at the top level as a dependency. (var svcA = new ServiceA(builderA); var svcA = new ServiceB(builderB)? A factory/builder doesn't have to be one globally shared object (and generally, one might argue that it shouldn't be - otherwise it's a ServiceLocator, which is sometimes considered an antipattern). Feb 9, 2022 at 18:54

1 Answer 1

1

In my opinion each alternative could be appropriate in some situations.

I would prefer the first example if all the properties are required, or if the factory is shared between multiple classes.

The second example might be appropriate if you are dealing with some complex configuration, but for this to make much sense the factory need to be temporary and not shared with anything. Having multiple classes share some hidden, mutable state would not be a good design.

The third example might be appropriate if there is complex configuration that are optional, and the factory needs to be shared. I.e. ServiceA just calls Build and is happy with any default configuration, while ServiceB wants to override some default values.

In both of the Builder examples I would probably recommend only using Add... methods for optional parameters, so the Build-method can be used without calling any additional methods. I.e. you might want to inject the IDbContext either in the constructor or as a parameter for the Build-method, since that presumably is required for the object to work at all.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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