Make DebugLogger usable by not stripping Debug.WriteLine

[NZSmartie]

Fixes #46, #58
Closes #123

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
As noted in #46 and #58, DebugLogger does not write to the debug output as Debug.WriteLine is stripped out in release builds of ReactiveUI.Splat.

ASPNet Core has a DebugLogger which works, so upon investigating, it was found that by including #define DEBUG in the source file, the compiler does not strip out the debug only functions. So this was copied over as shown in this PR.

What is the current behavior? (You can also link to an open issue here)
DebugLogger does nothing

What is the new behavior (if this is a feature change)?
DebugLogger writes to the debug output as intended

What might this PR break?
It should break nothing 😇

Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:

[ghuntley]

Thanks @NZSmartie. The CI will be red because we moved hosting recently. Can fix up later.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this because it helps reduce the workload of our extremely busy maintainers. If you want this issue progressed faster please start talks with a maintainer with how you can help out. There are so many ways to contribute to open-source and the most valued are often not code related. We are always looking for more help and look after those who stick around. Say howdy to one of the maintainers or browse https://reactiveui.net/contribute/ for ideas on where to start your journey.

Contribute

ReactiveUI is a composable, cross-platform model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming which is a paradigm that allows you to express the idea around a feature in one readable place, abstract mutable state away from your user interfaces and improve improve the testability of your application.

[NZSmartie]

Well, this is still good to merge right?

[Qonstrukt]

I'm not sure if it was meant to work this way. IMHO it would be much better to ask of a developer to implement their own DebugLogger which actually only works when running DEBUG code, as the name implies. No release application should log that much to the console by default. (There are exceptions of course.)

The DebugLogger supplied with Splat should be nothing more than a default implementation that can be useful while working on Splat itself. But I don't think it was ever meant for users of Splat to use by default.

[NZSmartie]

I could add a [Conditional("DEBUG")] attribute to the Write() method, which would do exactly that, only work on Debug builds and get stripped for other configurations.

[Qonstrukt]

But that would result in the same as it's working right now, or am I missing something? 😄
You're never calling this method directly from your own code, so the end result wouldn't change. But I might be wrong!

[NZSmartie]

Heh, it wont do the same, as the debug methods would be compiled into the splat library and be usable by developers that use it in their projects in with a configuration that defines DEBUG.

This is how it's currently done in ASP.Nets logging framework and is working quite well. This PR is literally taking their idea and using it in Splat.

[NZSmartie]

@NZSmartie commented on Sep 27
ASPNet Core's Debug Logger solves this by #define DEBUG as seen here
https://github.com/aspnet/Logging/blob/rel/2.0.0/src/Microsoft.Extensions.Logging.Debug/DebugLogger.debug.cs#L7

The catch is that the debug function itself is in a separate file to prevent unintended behaviour from happening that may be caused by DEBUG being present.

GitHub

aspnet/Logging

Logging - Common logging abstractions and a few implementations

[Qonstrukt]

Looking at the docs, and at the last paragraph in the answer here: https://stackoverflow.com/a/3788719/2782141 that would only work when you're calling the Write method yourself directly from your own code. When using the Log () extensions, this is never the case. So the Write method would still never be called when using those.

Looking at this from another perspective and hopefully being more helpful at the same time;
Would it be an alternative to introduce a ConsoleLogger implementation of ILogger which would always log to console? Irregardless of DEBUG being defined or not?

I get what you mean with your ASP.NET reference, but isn't logging configured per environment there? There it would be intended for a method to work irregardless of DEBUG flags, because you would enable or disable it depending on your environment.
A web application also has no visible console logging to it's end users, in UI applications this is entirely different and can cause serious security hazards.

So even though we could provide a ConsoleLogger, this still shouldn't be the default because it's a bad practice IMHO.

[glennawatson]

Thanks @NZSmartie I agree the current DebugLogger is not useful. Merging in now. Sorry for the long delay.

[glennawatson]

See https://github.com/reactiveui/splat/releases/tag/5.1.4 for release details.

GitHub

reactiveui/splat

A library to make things cross-platform that should be - reactiveui/splat