Access4Learning / sif3-framework-dotnet

SIF 3.0 Developer Framework for .NET
Apache License 2.0
14 stars 19 forks source link

Remove the Service Locator from GenericConsumer #2

Open aranm opened 10 years ago

aranm commented 10 years ago

GenericConsumer has a dependency on Log4Net. The logger is created as a static property in the GenericConsumer class. This violates good programming practice and needs to be changed for the following reasons:

  1. Dependencies should be injected. Classes should not instantiate their own dependencies. It makes unit testing hard and violates the (http://en.wikipedia.org/wiki/Dependency_inversion_principle)
  2. The ServiceLocator pattern has been used to resolve the logging framework. The service locator has been called an anti-pattern by some (http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/) But the real issue is that a service locator hides a classes dependencies. This is the case here.
  3. The logger should not be a singleton. When the framework is used in a multi-tenanted environment logging might want to be directed to different storage locations for different tenants. Having a singleton logger will not allow this as the configuration for the logger can only be specified once. Allowing each instance of GenericConsumer to have a different logger passed to the constructor would fix this issue.

What I would like to do:

  1. Create a logging interface that is for the framework. This would allow anyone to use the adaptor pattern (http://en.wikipedia.org/wiki/Adapter_pattern) to wrap whatever logger they prefer. A user could make their own and use Console.WriteLine or Trace.WriteLine for example.
  2. Add an extra constructor or a public property that can have the logger set on it. I would prefer a constructor parameter, but understand that people are using the framework already (technical debt) This would remove the service locator and allow logging to be flexible.
  3. Remove the log4Net nuget package.