5
\$\begingroup\$

I'm working with the unit of work pattern to glue together my application logic. To create such a unit of work object I use the builder pattern.

But my unit of work classes need access to the service collection of the dependency injection container but as the builder pattern requires you to know how to new the object this won't work. That's why I wrote the Build<T> method on MyUnitOfWork which uses reflection and the service container to get the parameters to construct the unit of work.

I know that I could remove MyUnitOfWorkConfiguration from the unit of work constructors but in the code I'm using this MyUnitOfWorkConfiguration class is quite big and the change would mean I need to duplicate the properties into the builder and the unit of work class.

public class MyUnitOfWorkConfiguration
{
 public bool RunSynchronously { get; set; }
}
public class SpecialUnitOfWorkConfiguration : MyUnitOfWorkConfiguration
{
 public bool IgnoreSpecialError { get; set; }
}
public abstract class MyUnitOfWork<TConfiguration> where TConfiguration : MyUnitOfWorkConfiguration
{
 protected readonly TConfiguration Configuration;
 protected MyUnitOfWork(IMyDependencyIncectedService myDependencyIncectedService, TConfiguration configuration)
 {
 // do something with the myDependencyIncectedService
 Configuration = configuration;
 }
 protected void ExecuteInner(Action action)
 {
 if (Configuration.RunSynchronously)
 action();
 else
 Task.Run(action);
 }
}
public class LoginUnitOfWork : MyUnitOfWork<MyUnitOfWorkConfiguration>
{
 public LoginUnitOfWork(IMyOtherDependencyIncectedService myOtherDependencyIncectedService, IMyDependencyIncectedService myDependencyIncectedService, MyUnitOfWorkConfiguration configuration)
 : base(myDependencyIncectedService, configuration)
 {
 // do something with the myOtherDependencyIncectedService
 }
 public void Login(string username, string password)
 {
 ExecuteInner(() =>
 {
 // do the login
 });
 }
}
public class SpecialUnitOfWork : MyUnitOfWork<SpecialUnitOfWorkConfiguration>
{
 public SpecialUnitOfWork(IMyDependencyIncectedService myDependencyIncectedService, SpecialUnitOfWorkConfiguration configuration) 
 : base(myDependencyIncectedService, configuration) { }
 public void Execute()
 {
 ExecuteInner(() =>
 {
 try { /* do special action */ }
 catch (SpecialException)
 {
 if (!Configuration.IgnoreSpecialError) throw;
 }
 });
 }
}
public abstract class MyUnitOfWorkBuilder<TConfiguration> where TConfiguration : MyUnitOfWorkConfiguration, new()
{
 protected readonly TConfiguration Configuration = new TConfiguration();
 public MyUnitOfWorkBuilder<TConfiguration> RunSynchronously()
 {
 Configuration.RunSynchronously = true;
 return this;
 }
 public T Build<T>() where T : MyUnitOfWork<TConfiguration>
 {
 var constructor = typeof(T).GetConstructors().First();
 var parameters = constructor.GetParameters();
 var parameterValues =
 parameters.Select(
 parameter =>
 parameter.ParameterType.IsAssignableFrom(typeof(MyUnitOfWorkConfiguration))
 ? Configuration
 : ServiceLocator.Current.GetInstance(parameter.ParameterType));
 return (T)constructor.Invoke(parameterValues.ToArray());
 }
}
public class MyUnitOfWorkBuilder : MyUnitOfWorkBuilder<MyUnitOfWorkConfiguration> { }
public class SpecialUnitOfWorkBuilder : MyUnitOfWorkBuilder<SpecialUnitOfWorkConfiguration>
{
 public SpecialUnitOfWorkBuilder IgnoreSpecialError()
 {
 Configuration.IgnoreSpecialError = true;
 return this;
 }
}
var loginUnitOfWork = new MyUnitOfWorkBuilder()
 .RunSynchronously()
 .Build<LoginUnitOfWork>();
loginUnitOfWork.Login("myUserName", "myPassword");
var specialUnitOfWork = new SpecialUnitOfWorkBuilder()
 .IgnoreSpecialError()
 .RunSynchronously()
 .Build<SpecialUnitOfWork>();
specialUnitOfWork.Execute();

Examples for such a unit of work are:

  • ExitApplicationUnitOfWork (logs out users, cleans up, asks if the user is sure etc.)
  • LoginUserUnitOfWork (logs the user in)
  • LogoutUserUnitOfWork (logs the user out)
  • BackupUnitOfWork (backs up stuff)

At the moment there are about 8 of those unit of work types and after the implementation of the new feature set there will be around 15.

My questions are

  • Is what I do bad practice?
  • Do I follow the rules of the builder, unit of work and dependency injection pattern? Or is what I did here something strange I came up with?^^
  • Could I solve this better?

P.S.: If you have any other inputs to my code please just let me know.

asked Apr 27, 2017 at 12:06
\$\endgroup\$
9
  • 2
    \$\begingroup\$ There are a lot of mys. This does not look like real code. \$\endgroup\$ Commented Apr 27, 2017 at 12:30
  • \$\begingroup\$ @t3chb0t the "my" is just a replacement of the products name \$\endgroup\$ Commented Apr 27, 2017 at 12:31
  • \$\begingroup\$ @t3chb0t and the SpecialUnitOfWork is an example \$\endgroup\$ Commented Apr 27, 2017 at 12:35
  • 1
    \$\begingroup\$ How many different configuration and 'unit of work' types do you have (or expect)? \$\endgroup\$ Commented Apr 27, 2017 at 20:04
  • 1
    \$\begingroup\$ @JanDotNet now there are 8. But after the implementation of the new featureset there will be around 15. \$\endgroup\$ Commented Apr 27, 2017 at 20:05

2 Answers 2

1
\$\begingroup\$

As t3chb0t mentioned in a comment, the code is hard to review because it contains only the the structural code without any functional stuff.

Maybe that is the reason that it looks a little bit over-engineered to me. I'll try it anyway.. however probably the review does not fit to your real code ;)

In my eyes, the builder has no functionality but providing a nice fluent API for initializing the configuration object and creating the UnitOfWork object. The second part requires the access to a service locator (with has actually nothing to do with dependency injection).

So, first of all I would drop the builder and create the configuration object manually (alternatively you could use the builder only for building the the configuration object and pass it to the unit of work in a separate step):

var config = new ConfigurationObjectA();
config.PropertyA = "Value";
...
var unitOfWork = new UnitOfWorkA(config, dependenyA, ....);

However, there is still the problem with the dependencies that must be injected manually. Depending on the DI Framework, you can use a factory for creating the UnitOfWorks with all it's dependencies. Ninject for example has an excellent Extensions providing exactly that by creating a simple interface:

public interface IUnitOfWorkFactory
{
 UnitOfWorkA CreateUnitOfWorkA(ConfigurationA config);
 UnitOfWorkB CreateUnitOfWorkB(ConfigurationB config);
 ....
}

Just register the interface as factory and you can use it for creating the unit of work objects. If the unit of work has additional constructor dependencies (which have to be registered in the DI container of course), Ninject injects these dependencies automatically.

That solution looks a) simpler with b) less code and c) without using a service locator ;)

answered Apr 27, 2017 at 20:56
\$\endgroup\$
7
  • \$\begingroup\$ but I would need to write a factory method for every unit of work right? so I exchange the reflection and the service locator with an factory. Its just less flexible^^ \$\endgroup\$ Commented Apr 27, 2017 at 20:59
  • \$\begingroup\$ and the builder pattern can't be dropped.. it's needed for all the default values and different configurations \$\endgroup\$ Commented Apr 27, 2017 at 21:00
  • \$\begingroup\$ You don't need to implement the interface - just define it and Ninject creates it's implementation "magically". \$\endgroup\$ Commented Apr 27, 2017 at 21:03
  • \$\begingroup\$ that sounds not to bad \$\endgroup\$ Commented Apr 27, 2017 at 21:04
  • \$\begingroup\$ but I'm using PRISM.. which doesn't want to relay on a specific dependency injection implementation.. as far as I remember \$\endgroup\$ Commented Apr 27, 2017 at 21:05
0
\$\begingroup\$

I am unable to leave comments due to the amount of reputation I have, so I have to resort to leaving a post.

Is what I do bad practice?

Yes. Service location is a well established anti-pattern. If you have to resort to using service location there is something wrong with the underlying design of your application.

answered Apr 27, 2017 at 20:00
\$\endgroup\$
1
  • 3
    \$\begingroup\$ I humbly recommend you to provide a complete example of a reworked version (since I myself was unable to come up with one that is less complex). It's not really helpful in this specific case to know it's a bad practice without knowing what the best practice would look like. \$\endgroup\$ Commented Apr 27, 2017 at 20:36

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.