better way to write wrapper class to get loose coupling

Question

class LogUtil<T> : ILogUtility
{
    log4net.ILog log;

    public LogUtil()
    {
        log = log4net.LogManager.GetLogger(typeof(T).FullName);
    }

    public void Log(LogType logtype, string message)
    {
        Console.WriteLine("logging coming from class {0} - message {1} " , typeof(T).FullName, message);
    }
}

public class Logger
{
    ILogUtility _logutility;

    public Logger(ILogUtility logutility)
    {
        _logutility = logutility;
    }


    public void Log(LogType logtype, string message)
    {
        _logutility.Log(logtype, message);
    }


}

I need to have the functionality to be flexible and have the ability to remove the LogUtil class in the future and use some thing else.

So I write LoggerUtility wrapper class as follows:

class LoggerUtility<T>
{
    public Logger logger
    {
        get
        {

            LogUtil<T> logutil = new LogUtil<T>();

            Logger log = new Logger(logutil);

            return log;
        }
    }
}

My client code as follows:

public class TestCode
{
    public void test()
    {

        new LoggerUtility<TestCode>().logger.Log(LogType.Info, "hello world");

    }

}

I am coding Logger property which may not be clean.

as you can see that following line does not look clean.

new LoggerUtility<TestCode>().logger.Log(LogType.Info, "hello world");

Is there a better way to write the client code? I want to have loose coupling with LogUtil and not use it directly in my client code.

Please let me know.

Thanks

Why not just code against the `ILogUtil` interface implemented by `LogUtil`? — Lee, Apr 24 '12 at 19:30
Have clients of the logger depend on `ILogUtil` instead of `LogUtil` i.e. `ILogUtil logger = GetLogger();` instead of `LogUtil = GetLogger();`. Then you can change the implementation if you need to. Alternatively you could just depend on log4net directly (or its `ILog` interface) since you're unlikely to need to change your logger implementation. — Lee, Apr 24 '12 at 21:06

score 2 · Accepted Answer · edited May 23 '17 at 10:34

The answer provided in comments is correct (clients should depend on the interface ILogUtil rather than a concrete implementation directly). There are a myriad of other problems also:

You're instantiating a new instance of the LoggerUtility<T> class and the Logger class every time you log a message. Perhaps something here should be static? What is the point of the extra layer (LoggerUtility)?
Your use of generics (LoggerUtility<T>) doesn't entirely make sense, since you're not bound to just type T, and you don't make use of that information.

In reality, writing your own logging facade is effort that other people have already spent - just use an existing implementation. I could vouch for both log4net and NLog, but if you'd prefer to have flexibility go for a proper facade in Castle.Services.Logging, which has adapters for the previous mentioned implementations (and you could write your own!).

More info here: Is there a logging facade for the .NET world?

score 0 · Answer 2 · answered Sep 06 '17 at 08:24

Depends on how sophisticated you want your logging wrapper to behave?

There are multiple levels to logging, Information and Exceptions are the norm.

The answers revolving around using Interfaces is 100% correct, but there is also the principal of DRY (Don't Repeat Yourself).

If you find that your code is looking very repetitive, as mine was, then maybe on top of using the standards such as Injection and Interfaces, also implement a Generic wrapper around the handling of errors.

Generics allow you to separate the logic for the solution and allow for reuse, Interfaces allow you to decouple the concepts of logging from the physical implementation.

  public static output ExecuteBlockwithLogging<output, input, config>(ExeBlock<output, input, config> exeBlock, input InputForExeBlock, ILoggingBlock logger)
    {

        exeBlock.Execute(InputForExeBlock);

        if ((exeBlock.logEntries != null) && (exeBlock.logEntries.Length > 0))
        {
            logger.Execute(exeBlock.logEntries);
        }


        if ((exeBlock.exceptions != null) && (exeBlock.exceptions.Length > 0))
        {
            foreach (var e in exeBlock.exceptions)
            {

                var dictionaryData = new Dictionary<string, string>();
                if (e.Data.Count > 0)
                {
                    foreach (DictionaryEntry d in e.Data)
                    {
                        dictionaryData.Add(d.Key.ToString(), d.Value.ToString());
                    }
                }

                var messages = e.FromHierarchy(ex => ex.InnerException).Select(ex => ex.Message);


                LoggingEntry LE = new LoggingEntry
                {
                    description = e.Message,
                    exceptionMessage = String.Join(Environment.NewLine, messages),
                    source = exeBlock.GetType().Name,
                    data = dictionaryData
                };

                logger.Execute(new LoggingEntry[] { LE });
            }
            return default(output);
        }

        return exeBlock.Result;
    }

better way to write wrapper class to get loose coupling

2 Answers2