Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetworkEventsManager dependency on the ILogger limits posibility to use it elsewhere #25

Open
mpostol opened this issue Aug 14, 2020 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@mpostol
Copy link
Collaborator

mpostol commented Aug 14, 2020

According to the architecture (description covered by the https://commsvr.gitbook.io/ooi/reactive-communication/semanticdata) implementation of the DataRepository provides it own logging infrastructure. The presented implementation requires ILogger{t}. It limits the possibility to reuse this library in case the main application doesn't implement this particular interface. It raises a question:

  • if the end-user program should be adopted to use this implementation as an external apart
  • if the library should be adopted to meet the base library requirements.

The library requirements are defined in the document Getting Started Tutorial and this implementation doesn't follow this description. It raises typical "chicken/egg - which one was the first" problem and therefore must be recognized as a critical error.

@mpostol mpostol added the bug Something isn't working label Aug 14, 2020
@Drutol
Copy link
Owner

Drutol commented Aug 24, 2020

This is indeed "chicken/egg - which one was the first" problem.

It's the same case as with ServiceLocator, we have to agree on some common abstraction that becomes "a standard" even if it remains informal. I've decided upon using Microsoft.Extensions.Logging.Abstractions nuget which with 413 millions of downloads became such an informal standard in a dotnet ecosystem. The key part is Abstractions it does not contain any kind of implementaion, it's just a set of contracts the successfully exhausts usual logging needs and has aquired support from many different libraries providing implementations.

Also please notice how the ILogger argument is an optional one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants