Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Cookies middleware should not enforce need for ILoggerFactory #4

Closed
Praburaj opened this issue Apr 11, 2014 · 3 comments
Closed

Cookies middleware should not enforce need for ILoggerFactory #4

Praburaj opened this issue Apr 11, 2014 · 3 comments

Comments

@Praburaj
Copy link
Contributor

Currently when ILoggerFactory is not registered in the IoC container the Cookies middleware throws a null reference exception as this is a compulsory parameter. This has to be an optional parameter if the app does not want to do any logging.

@Tratcher
Copy link
Member

  1. If we inject ILoggerFactory via DI, you can't make it optional.
  2. Hosting should provide an implementation, even if it's a NullLoggerFactory that returns a NullLogger. Individual middleware shouldn't have to deal with this.
  3. Consider defining NullLoggerFactory in the Logging assembly.

@Eilon
Copy link
Member

Eilon commented Apr 15, 2014

Yeah I'm not sure whether we want to use a null object pattern here or not. There are certainly both pros and cons (having to write null checks vs. unnecessary calls into a black hole).

As far as being optional with DI, there are two ways to achieve this:

  1. Instead of injecting, use GetService (and catch exceptions if it throws)
  2. Take in an IEnumerable<ILoggerFactory> and check that you got either 0 or 1 back.

I'm sure that within minutes both @lodejard and @davidfowl will read this and have simultaneous heart attacks 💔 💔 but I'm curious what they'll say.

@Praburaj
Copy link
Contributor Author

I moved this bug to Hosting repo as it make sense to have this ILoggerFactory created in hosting. Here is the bug in hosting : aspnet/Hosting#20

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants