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?
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).