sventevit
sventevit

Reputation: 4816

How to disable evaluation of parameters for log method calls

My application contains a lot of logging statements, something like this:

var logger = new Logger(/*..get flag from settings if the logger should be active..*/);
// ....
logger.LogActivity(..serialize an object..);
//...
logger.LogActivity(..get another object's expensive overriden ToString method..);
//...
logger.LogActivity(..log something new..);

Logger class:

public class Logger
{
  private readonly bool _isActive;
  
  public Logger(bool isActive)
  {
    _isActive = isActive;
  }
  
  public void LogActivity(string activity)
  {
    if (_isActive)
    {
      // Save activity to Database.
    }
  }
}

When I disable logger in settings (so the _isActive field in Logger class is false), then nothing is saved to database.

But all the expressions in the Logger.LogActivity methods are still evaluated (for instance ..serialize object.. in previous example) and this slows down my application.

I could use log statements like this:

var logger = new Logger(/*..get flag from settings if the logger should be active..*/);
// ....
if (loggerIsActive) logger.LogActivity(..serialize an object..);
//...
if (loggerIsActive) logger.LogActivity(..get another object's expensive overriden ToString method..);
//...
if (loggerIsActive) logger.LogActivity(..log something new..);

But for me it would be best to change only my LogActivity method somehow.

Is it possible to modify the Logger class so that when logging gets disabled (or log level changes), the expressions in LogActivity(...) calls aren't evaluated?

Is there any pattern to do this in C#?

Upvotes: 5

Views: 1041

Answers (3)

sɐunıɔןɐqɐp
sɐunıɔןɐqɐp

Reputation: 3542

A good way to avoid parameter evaluation is to use a conditional expression before calling your log method.


But I really don't like the following uglyness:

public enum LogLevel
{
    Debug,
    Info,
    Warning,
    Error,
    Failure,
    None,
}

if(Logger.LogLevel <= LogLevel.Debug)
    Logger.Write(LogLevel.Debug, $"{complexStuffToBeEvaluatedAsString}");

There must be a better solution...

Suppose you have a ProxyLogger for logging to a specific LogLevel:

public sealed class ProxyLogger
{
    internal LogLevel Level { get; }
    internal Logger Logger { get; }
    
    internal ProxyLogger(Logger logger, LogLevel level)
    {
        Logger = logger;
        Level = level;
    }
}

public static class ProxyLoggerExtensions
{
    public static ProxyLogger Write(this ProxyLogger proxyLogger, object o)
    {
        proxyLogger?.Logger?.Write(proxyLogger.Level, o);
        return proxyLogger;
    }
}

And suppose you have 5 instances of this proxy logger class as properties of your main logger class (one proxy class for each LogLevel):

public sealed class Logger
{
    public ProxyLogger Debug { get; private set; }
    public ProxyLogger Info { get; private set; }
    public ProxyLogger Warn { get; private set; }
    public ProxyLogger Error { get; private set; }
    public ProxyLogger Critical { get; private set; }

    private LogLevel _Level;
    public LogLevel Level
    {
        get => _Level;
        set
        {
            _Level = value;
            Debug = value > LogLevel.Debug ? null : new ProxyLogger(this, LogLevel.Debug);
            Info = value > LogLevel.Info ? null : new ProxyLogger(this, LogLevel.Info);
            Warn = value > LogLevel.Warn ? null : new ProxyLogger(this, LogLevel.Warn);
            Error = value > LogLevel.Error ? null : new ProxyLogger(this, LogLevel.Error);
            Critical = value > LogLevel.Critical ? null : new ProxyLogger(this, LogLevel.Critical);
        }
    }

    public Logger(LogLevel level = LogLevel.Info) =>
        Level = level;

    public Logger Write(LogLevel level, object o)
    {
        Console.WriteLine($"{level}: {o}"); // or whatever you wish...
        return this;
    }
}

Then you could completely avoid parameter evaluation by checking whether the proxy logger for that specific LogLevel is instantiated by using the question mark symbol ?

public static class Program
{
    // You could use dependency injection instead of this:
    private static Logger Logger { get; } = new(LogLevel.Info);

    internal static void Main()
    {
        // 'Info' proxy logger is instantiated: normal logging happens
        Logger.Info?.Write("Logging with Logger.Info?.Write(...)");

        // 'Debug' proxy logger is NOT instantiated: param evaluation won't happen
        Logger.Debug?.Write("Logging with Logger.Debug?.Write(...)");

        // You forgot the question mark => param evaluation happens
        // No exception is thrown since 'Write()' is an extension method
        // Nothing will be logged because 'Debug' level is deactivated
        Logger.Debug.Write("Logging with Logger.Debug.Write(...)");

        // Multiline:
        Logger.Info?
            .Write("Multiple")
            .Write("calls");

        Logger.Debug?
            .Write("These parameters")
            .Write("won't be evaluated")
            .Write("nor logged");
    }
}

In case you forget to use the question mark symbol, you would ask yourself whether a NullReferenceException is thrown... No, it won't happen, because you just called Write(...) as an extension method, which has it's own internal null check.

The downside of this approach is: if you forget to use the question mark when logging, then parameter evaluation will still happen. Maybe the question mark could be enforced by a code analyser?

Otherwise, it works as intended.

Upvotes: 1

zmbq
zmbq

Reputation: 39079

You can't avoid the parameter evaluation altogether, no, but you can provide some additional logging methods that can help you out here.

@Martin and @kenny suggested passing lambda expressions. This will defer the evaluation of the code, but will result in ugly client code. You might save a few cycles, but I doubt it would be worth it.

I suggest you create several LogActivity overrides:

LogActivity(string message) // logs the message
LogActivity(object obj) // calls obj.ToString() and logs it

And keep the lambda expression variant, if you think it's really necessary:

LogActivity(Func<String> func) // evaluates the function and logs its response

Note, you won't be able to avoid the cost of calling LogActivity. C# isn't C, and you don't have macros that work the way they do in C. That's hardly a problem, if calling a non-virtual method is expensive for you, you're doing it so often you shouldn't place log messages in there, anyway.

Upvotes: 2

Martin
Martin

Reputation: 181

You could add an overload that takes a Func<string> that would generate the string to be logged.

public void LogActivity(Func<string> activity)
{
    if (_isActive)
    {
        string log = activity();
        // save 'log' to database
    }
}

Then use it like this:

logger.LogActivity(() => expensiveObject.ToString());

Upvotes: 4

Related Questions