views:

564

answers:

8

I've got a logging class which requires need to be called from almost everywhere in the application.

However it requires setup in the beginning of the application with "which path to write", "log level" and if it's "enabled" or not.

I don't want to give this parameters every time or pass Logging class as parameter to every single object in my application, so I do use singleton pattern for logging.

Recently I've suffered a lot from tightly coupled classes I don't want to do the same mistake again but after thinking about this sounds like this is the only good solution.

UPDATE :

I don't really care about logging what I care is solving similar design issues, I'm having the same dilemma with another global settings object which requires to be used from so many classes. But injecting it into every single of them just makes a horrible overhead and less readable code.

What do you think about this implementation and what do you do when you come across similar design decisions?

P.S. Please do not suggest something like "use Log4X library" etc.

+4  A: 

First - could you write the log writer as a trace listener, and use Trace.Write etc from the methods?

Do you actually need an instance here? That would be useful, for example, if you wanted to abstract it as a TextWriter or similar - but if it is going to be a standalone singleton, can the methods not use static methods directly, i.e. Log.Write(...) (rather than passing in a log instance)?

Re the general problem - it depends on the types that are doing the logging. For "manager" (etc) classes, you might consider using dependency injection (Unity, StructureMap, etc) to automate this. I wouldn't normally use injection with DTOs, though.

Marc Gravell
of course. plus one.
Adeel Ansari
DoH! You are quicker than I!
Chuck Conway
I need to set where to write the log and if the log is enabled or not. So I need an instance to set these options. I think DI might help me I'll look into it.
dr. evil
Why does that demand an instance? You can use either configuration (in a static ctor), or you can use a static (synchronized) `SetLogOptions(...)` method, that you call early on in your app...
Marc Gravell
@Marc you absolutely right, I was being a muppet. I've just changed the design to use a statics as settings and changed all others to statics as well. Somehow I convinced myself that I should use an instance to store settings (I can't use persistent because user need to change settings on the fly).
dr. evil
+3  A: 

Even if you don't want suggestions of "use Log4X" (although you don't say exactly why you want to reinvent the wheel) it would seem sensible to look at the design decisions made by various logging libraries.

In my experience the problems of tight coupling aren't as relevant when applied to logging - in particular, I rarely want to test the logging side of my app, and I don't mind if it logs to the console during unit testing.

In short, the "normal" pattern of:

private static readonly Logger log = LogManager.GetLogger(...);

(with appropriate name changes etc) is aesthetically unappealing in its use of static methods, but works pretty well in practice. At least, that's been my experience.

Jon Skeet
@Jon idea is not reinventing wheel getting a generic sense of solution for similar problems. Logging is an obvious problem, so I thought it'd be nice to figure how tackle these problems. Thanks for static usage it looks better than singleton.
dr. evil
The trouble is that there aren't many problems which are similar to logging, in my experience. Almost anywhere else, I'd want to use DI - but as logging shouldn't affect the rest of the behaviour of the code, it's relatively harmless to take the simpler approach.
Jon Skeet
You mean code has value beyond aesthetics? Heresy!
CurtainDog
A: 

Is this ASP.Net? If so, you can use The Error Event in the Global.asax.

For your many dependencies, have you considered using a dependency injection framework?

Update

I'm not sure the performance implication nor how relevant performance is to your app, but this framework looks interesting: PostSharp, A blog post about it.

You also might be able to leverage the Conditional attribute.

If you use PostSharp, I would be interested how it works out.

Chuck Conway
This is Winforms, I didn't want to introduce a DI due to the implementation overhead but maybe I should.
dr. evil
+1  A: 

You can probably use a singleton here. You'll have a tight coupling between every class in the application and the logger class but if the logger class and the global settings class really are needed in every class this can be acceptable.

J W
A: 

Logging and settings are actually handled in two different ways, so if I understood correctly, your actual question was more related to handling global settings between assemblies.

Regarding logging, things are pretty clear - using a global Singleton for that is common, although it does tightly couple your libraries to the log library. Using Trace listeners is an even better solution IMHO.

But when talking about application settings, you should certainly avoid making them global. Keep all application related settings located at one place only (those that should be persisted), but not statically available to other libraries. Therefore, passing appropriate settings to other assemblies will have to be the caller's responsibility, not vice versa.

Groo
When it becomes caller's responsibility and pass it to 30 other classes isn't a bit like overkilling? I mean then code will includes lots of parameters for no good reason and almost for no extra benefit. -this is question more than a statement :) -
dr. evil
If you plan to reuse your libraries then it's not so nice to have references to static objects in other assemblies. If each of the classes need the same settings code (or a large part of it), than it may indicate that the code itself is tightly coupled already.
Groo
+1  A: 

I personally use static class in such case. The class has static configuration fields (for manual experimenting) plus some functions to populate them using configurtion from the appropriate .config file section.

This is in effect preatty much close to what would you have with DI as you can "inject" new config. To change configuration to the new model, I just change in .config file field that keeps the "active" configuration section.

This is easy to use, easy to maintain, and everybody understands it... I don't see any particular drawback of it...

majkinetor
+1 Spot on. Exactly what I do.
Finglas
A: 

One thing you could investigate is package-by-feature. It is claimed that following this technique alliviates some issues that result in high coupling between classes. More specifically it would mean that there would only be one class in every feature in your application with the responsibility of talking to the configuration provider (which could well be part of a configuration/setup/install feature itself). The level of coupling is still on the high-ish side, but because it is well-defined it should be managable.

CurtainDog
A: 

Something similar:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using log4net;
using log4net.Config;
using log4net.Appender;
using System.Reflection;
using System.IO;
using System.Globalization;
using log4net.Core;
using System.Web;

namespace GenApp.Utils
{
  ///<summary> Wrapper around log4net with dynamically adjustable verbosity</summary>
  public class Logger
  {

    private static Logger inst = new Logger ();
    public static Logger Inst ()
    {
      inst.ConfigureLogging ();
      return inst;
    }


    public enum DebugLevel : int
    {
      Fatal_Msgs = 0,
      Fatal_Error_Msgs = 1,
      Fatal_Error_Warn_Msgs = 2,
      Fatal_Error_Warn_Info_Msgs = 3,
      Fatal_Error_Warn_Info_Debug_Msgs = 4
    }

    public static void Debug ( GenApp.Bo.User objUser, ILog logger, string msg )
    {
      DebugLevel debugLevel = (DebugLevel)objUser.UserSettings.LogLevel;
      string strLogLevel = Logger.GetLogTypeString ( debugLevel );
      inst.SetLogingLevel ( strLogLevel );
      logger.Debug ( msg );

    } //eof method 


    public static void Info ( GenApp.Bo.User objUser, ILog logger, string msg )
    {
      DebugLevel debugLevel = (DebugLevel)objUser.UserSettings.LogLevel;
      string strLogLevel = Logger.GetLogTypeString ( debugLevel );
      inst.SetLogingLevel ( strLogLevel );
      logger.Info ( msg );

    } //eof method 


    public static void Warn ( GenApp.Bo.User objUser, ILog logger, string msg )
    {
      DebugLevel debugLevel = (DebugLevel)objUser.UserSettings.LogLevel;
      string strLogLevel = Logger.GetLogTypeString ( debugLevel );
      inst.SetLogingLevel ( strLogLevel );
      logger.Warn ( msg );

    } //eof method 


    public static void Error ( GenApp.Bo.User objUser, ILog logger, string msg )
    {
      DebugLevel debugLevel = (DebugLevel)objUser.UserSettings.LogLevel;
      string strLogLevel = Logger.GetLogTypeString ( debugLevel );
      inst.SetLogingLevel ( strLogLevel );
      logger.Error ( msg );
    } //eof method 


    public static void Fatal ( GenApp.Bo.User objUser, ILog logger, string msg )
    {
      DebugLevel debugLevel = (DebugLevel)objUser.UserSettings.LogLevel;
      string strLogLevel = Logger.GetLogTypeString ( debugLevel );
      inst.SetLogingLevel ( strLogLevel );
      logger.Fatal ( msg );
    } //eof method 


    /// <summary>
    /// Activates debug level 
    /// </summary>
    /// <sourceurl>http://geekswithblogs.net/rakker/archive/2007/08/22/114900.aspx&lt;/sourceurl&gt;
    private void SetLogingLevel ( string strLogLevel )
    {

      this.ConfigureLogging ();
      string strChecker = "WARN_INFO_DEBUG_ERROR_FATAL";

      if (String.IsNullOrEmpty ( strLogLevel ) == true || strChecker.Contains ( strLogLevel ) == false)
        throw new ArgumentOutOfRangeException ( " The strLogLevel should be set to WARN , INFO , DEBUG ," );



      log4net.Repository.ILoggerRepository[] repositories = log4net.LogManager.GetAllRepositories ();

      //Configure all loggers to be at the debug level.
      foreach (log4net.Repository.ILoggerRepository repository in repositories)
      {
        repository.Threshold = repository.LevelMap[strLogLevel];
        log4net.Repository.Hierarchy.Hierarchy hier = (log4net.Repository.Hierarchy.Hierarchy)repository;
        log4net.Core.ILogger[] loggers = hier.GetCurrentLoggers ();
        foreach (log4net.Core.ILogger logger in loggers)
        {
          ( (log4net.Repository.Hierarchy.Logger)logger ).Level = hier.LevelMap[strLogLevel];
        }
      }

      //Configure the root logger.
      log4net.Repository.Hierarchy.Hierarchy h = (log4net.Repository.Hierarchy.Hierarchy)log4net.LogManager.GetRepository ();
      log4net.Repository.Hierarchy.Logger rootLogger = h.Root;
      rootLogger.Level = h.LevelMap[strLogLevel];
    }

    ///<summary>
    ///0 -- prints only FATAL messages 
    ///1 -- prints FATAL and ERROR messages 
    ///2 -- prints FATAL , ERROR and WARN messages 
    ///3 -- prints FATAL  , ERROR , WARN and INFO messages 
    ///4 -- prints FATAL  , ERROR , WARN , INFO and DEBUG messages 
    ///</summary>
    private static string GetLogTypeString ( DebugLevel debugLevel )
    {

      string srtLogLevel = String.Empty;
      switch (debugLevel)
      {
        case DebugLevel.Fatal_Msgs:
          srtLogLevel = "FATAL";
          break;
        case DebugLevel.Fatal_Error_Msgs:
          srtLogLevel = "ERROR";
          break;
        case DebugLevel.Fatal_Error_Warn_Msgs:
          srtLogLevel = "WARN";
          break;
        case DebugLevel.Fatal_Error_Warn_Info_Msgs:
          srtLogLevel = "INFO";
          break;
        case DebugLevel.Fatal_Error_Warn_Info_Debug_Msgs:
          srtLogLevel = "DEBUG";
          break;
        default:
          srtLogLevel = "FATAL";
          break;
      } //eof switch
      return srtLogLevel;

    } //eof GetLogTypeString


    /// <summary>
    /// The path where the configuration is read from.
    /// This value is set upon a call to ConfigureLogging().
    /// </summary>
    private string configurationFilePath;
    public void ConfigureLogging ()
    {
      lock (this)
      {
        bool configured = false;


        #region ConfigureByThePathOfTheEntryAssembly
        // Tells the logging system the correct path.
        Assembly a = Assembly.GetEntryAssembly ();

        if (a != null && a.Location != null)
        {
          string path = a.Location + ".config";

          if (File.Exists ( path ))
          {
            log4net.Config.DOMConfigurator.Configure (
              new FileInfo ( path ) );
            configurationFilePath = path;
            configured = true;
          }
          else
          {
            path = FindConfigInPath ( Path.GetDirectoryName ( a.Location ) );
            if (File.Exists ( path ))
            {
              log4net.Config.DOMConfigurator.Configure (
                new FileInfo ( path ) );
              configurationFilePath = path;
              configured = true;
            }
          }
        }
        #endregion ConfigureByThePathOfTheEntryAssembly


        #region ConfigureByWeb.config
        // Also, try web.config.
        if (!configured)
        {
          if (HttpContext.Current != null &&
            HttpContext.Current.Server != null &&
            HttpContext.Current.Request != null)
          {
            string path = HttpContext.Current.Server.MapPath (
              HttpContext.Current.Request.ApplicationPath );

            path = path.TrimEnd ( '\\' ) + "\\Web.config";

            if (File.Exists ( path ))
            {
              log4net.Config.DOMConfigurator.Configure (
                new FileInfo ( path ) );
              configurationFilePath = path;
              configured = true;
            }
          }
        }
        #endregion ConfigureByWeb.config


        #region ConfigureByThePathOfTheExecutingAssembly
        if (!configured)
        {
          // Tells the logging system the correct path.
          a = Assembly.GetExecutingAssembly ();

          if (a != null && a.Location != null)
          {
            string path = a.Location + ".config";

            if (File.Exists ( path ))
            {
              log4net.Config.DOMConfigurator.Configure (
                new FileInfo ( path ) );
              configurationFilePath = path;
              configured = true;
            }
            else
            {
              path = FindConfigInPath ( Path.GetDirectoryName ( a.Location ) );
              if (File.Exists ( path ))
              {
                log4net.Config.DOMConfigurator.Configure (
                  new FileInfo ( path ) );
                configurationFilePath = path;
                configured = true;
              }
            }
          }
        }
        #endregion ConfigureByThePathOfTheExecutingAssembly


        #region ConfigureByThePathOfTheCallingAssembly
        if (!configured)
        {
          // Tells the logging system the correct path.
          a = Assembly.GetCallingAssembly ();

          if (a != null && a.Location != null)
          {
            string path = a.Location + ".config";

            if (File.Exists ( path ))
            {
              log4net.Config.DOMConfigurator.Configure (
                new FileInfo ( path ) );
              configurationFilePath = path;
              configured = true;
            }
            else
            {
              path = FindConfigInPath ( Path.GetDirectoryName ( a.Location ) );
              if (File.Exists ( path ))
              {
                log4net.Config.DOMConfigurator.Configure (
                  new FileInfo ( path ) );
                configurationFilePath = path;
                configured = true;
              }
            }
          }
        }
        #endregion ConfigureByThePathOfTheCallingAssembly


        #region ConfigureByThePathOfTheLibIsStored
        if (!configured)
        {
          // Look in the path where this library is stored.
          a = Assembly.GetAssembly ( typeof ( Logger ) );

          if (a != null && a.Location != null)
          {
            string path = FindConfigInPath ( Path.GetDirectoryName ( a.Location ) );
            if (File.Exists ( path ))
            {
              log4net.Config.DOMConfigurator.Configure (
                new FileInfo ( path ) );
              configurationFilePath = path;
              configured = true;
            }
          }
        }
        #endregion ConfigureByThePathOfTheLibIsStored



      } //eof lock   
    } //eof method 



    /// <summary>
    /// Searches for a configuration file in the given path.
    /// </summary>
    private string FindConfigInPath (
      string path )
    {
      string[] files = Directory.GetFiles ( path );

      if (files != null && files.Length > 0)
      {
        foreach (string file in files)
        {
          if (Path.GetExtension ( file ).Trim ( '.' ).ToLower (
            CultureInfo.CurrentCulture ) == "config")
          {
            return file;
          }
        }
      }

      // Not found.
      return string.Empty;
    } //eof method 



    /// <summary>
    /// Remove dynamically appenders
    /// </summary>
    /// <param name="appenderName"></param>
    /// <param name="threshold"></param>
    public static void SetThreshold ( string appenderName, Level threshold )
    {
      foreach (AppenderSkeleton appender in LogManager.GetRepository ().GetAppenders ())
      {
        if (appender.Name == appenderName)
        {
          appender.Threshold = threshold;
          appender.ActivateOptions ();

          break;
        }
      }
    } //eof method 



  } //eof class 


} //eof namespace
YordanGeorgiev