HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Proper separation of concerns for stateful logger

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
loggerconcernsstatefulforproperseparation

Problem

This is a code review for a set of loggers following this question.
The project is a universal windows application targeting Windows Phone.

There are concerns over the tight coupling between the loggers and App. There is way too much code in the main program that is responsible for setting up the logger just right.

There are also concerns on how to deal with the fact that a ReleaseLogger requires special behavior (establishing a session) that the DebugLogger class does not, and how to deal with this.

App.xaml.cs

```
using Log.Common;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.WindowsRuntime;
using System.Threading.Tasks;
using Windows.ApplicationModel;
using Windows.ApplicationModel.Activation;
using Windows.Foundation;
using Windows.Foundation.Collections;
using Windows.Foundation.Diagnostics;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Controls.Primitives;
using Windows.UI.Xaml.Data;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Media;
using Windows.UI.Xaml.Media.Animation;
using Windows.UI.Xaml.Navigation;

// The Blank Application template is documented at http://go.microsoft.com/fwlink/?LinkId=234227

namespace Log
{
///
/// Provides application-specific behavior to supplement the default Application class.
///
public sealed partial class App : Application
{
#if WINDOWS_PHONE_APP
private TransitionCollection transitions;
#endif

public const string TelemetryUrl = "http://try.count.ly";
public const string TelemetryAppKey = "";

private ILogger _logger;
private ILogger Logger
{
get
{
if(null == _logger)
{
//#if PROD
_logger = new ReleaseLogger();
//#else
//_logger = new DebugLogger();
//#endif
}
return _logger;
}
}

///

Solution

General advice for decoupling logging from application code:

Have your logger listen for events instead of injecting your logger into application code. This inverts the dependency so that business code doesn't need to even know that a logger exists.

As for the amount of code related to logging in your App class, I don't see it as something to be concerned about. If there's anywhere in a UWP app to wire these kinds of things up, this is the place.

One quick, unrelated, note about async. You do this several times.

public void Log(string message, LoggingLevel loggingLevel, params object[] args)
    {
        var result = LogAsync(message, loggingLevel, args);
    }


Wrapping asynchronous methods in synchronous void methods defeats the purpose of having async methods to begin with. If you're going to use async/await, you really need to "Go async all the way". I'd highly recommend reading Stephen Cleary's article "Async/Await - Best Practices in Async Programming".

Code Snippets

public void Log(string message, LoggingLevel loggingLevel, params object[] args)
    {
        var result = LogAsync(message, loggingLevel, args);
    }

Context

StackExchange Code Review Q#150528, answer score: 2

Revisions (0)

No revisions yet.