U4-5231 - LogHelper violates DIP - new ILogger interface

Created by Lars-Erik Aabech 16 Jul 2014, 07:13:27 Updated by Shannon Deminick 09 Jan 2015, 07:42:19

LogHelper should at least wrap an instance of an interface, like the other "resolvers". Currently, it's tightly coupled to log4net and any testable code will have to adapt it to leave the dependency out.

Comments

Anthony 20 Nov 2014, 16:14:34

Done!

https://github.com/umbraco/Umbraco-CMS/pull/559


Anthony 21 Nov 2014, 01:13:52

I guess I should have explained a bit here...

Created an interface based on the LogHelper, and created a service, exposed through Application.Services. Made LogHelper obsolete.

It makes much more sense now, as it is in line with the other services. So now a developer can access the logger via Application.Services.LoggingService

Since this is interfaced, a developer can now pass around an ILoggingService, which makes logging in their logic unit testable, with no dependency on log4net.


Shannon Deminick 21 Nov 2014, 02:07:19

I fully understand that Anth :-) as an interface it needs to be simpler, there is zero need to have any generic methods on it, these can be extension methods and still work with mocking.


Lars-Erik Aabech 21 Nov 2014, 06:17:39

All good. H5YR, guys. :)


Shannon Deminick 21 Nov 2014, 06:20:38

There's some comments in the PR, if you don't get time to do it i can do it later on but this is gonna have to wait till 7.3 i think, we'll see. Also, if we are going to obsolete things in the core, we should really try to update all references in the core


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Easy

Category: Architecture

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions:

Due in version: 7.3.0

Sprint:

Story Points:

Cycle: