The way I have done this is to create a wrapper class for NLog, which will wrap each log method, and obuscate methodName away and use the StackTrace object to get the method name. Then you don't have to write it every time; the method name of the method that calls the Logging wrapper method is injected automatically.
It will look cleaner as you won't have {0} and methodName everywhere.
You can take it even one step further, and create a logging wrapper class that takes the log string and an Action, executes the Action, and calls the log object using the StackTrace object all in one shot.
I've used this for executing time actions and logging them, it is convenient to do all in one call and saves on repetitive code. My method, ExecuteTimedAction(string logString, Action actionToExecute) uses a stopwatch, logs a start string, starts stopwatch, executes the method (Action delegate), stops the stopwatch, and logs again with both logs having a time stamp, name of the assembly, and name of the method that the call initiated from.
The code for getting the method is simple, use the StackTrace object, and get the StackFrame of the previous call.
var stackTrace = new StackTrace();
var callingMethodName = stackTrace.GetFrame(2).GetMethod().Name;
Note I have 2 hardcoded above but that is because of an additional wrapper call; if you are calling directly, then you may need GetFrame(1) instead. Best way is to use immediate window and try out the different frames, or just loop through it to see what you get, using GetFrames() method of StackTrace object.
I'm looking now into keeping the params for the string format and appending the first Param for the log wrapper. It can be done something like this:
public static class LogWrapper
{
private static Logger _logger // where Logger assumes that is the actual NLog logger, not sure if it is the right name but this is for example
public static void Info(string logString, object[] params)
{
// Just prepend the method name and then pass the string and the params to the NLog object
_logger.Info(
string.Concat(
GetMethodName(),
": ",
logString
),
params
);
}
public static void Warn(string logString, object[] params)
{
// _logger.Warn(
// You get the point ;)
// )
}
private static string GetMethodName()
{
var stackTrace = new StackTrace(); // Make sure to add using System.Diagnostics at the top of the file
var callingMethodName = stackTrace.GetFrame(2).GetMethod().Name; // Possibly a different frame may have the correct method, might not be 2, might be 1, etc.
}
}
Then in your calling code, the _logger member becomes LoggerWrapper, not Logger, and you call it exactly the same way but you remove the {0} from the code. You'd need to check for nulls and maybe if there are no other params, have a method overload that just calls without the params; I am not sure if NLog supports that, so you have to check this.
... EDIT:
Just for point of interest I use this type of code in common library type of assemblies that might be referenced by a bunch of assemblies, so I can get the information such as calling assembly, method name, etc., without hardcoding it or worrying about it in my logging code. It also makes sure anyone else using the code doesn't have to worry about it. They just call Log() or Warn() or whatever and the assembly is automatically saved in the logs.
Here's an example (I know you said overkill for you but food for thought for the future if you might need something like this). In this example, I was only logging the assembly, not the method name but it can easily be combined.
#region : Execute Timed Action :
public static T ExecuteTimedAction<T>(string actionText, Func<T> executeFunc)
{
return ExecuteTimedAction<T>(actionText, executeFunc, null);
}
/// <summary>
/// Generic method for performing an operation and tracking the time it takes to complete (returns a value)
/// </summary>
/// <typeparam name="T">Generic parameter which can be any Type</typeparam>
/// <param name="actionText">Title for the log entry</param>
/// <param name="func">The action (delegate method) to execute</param>
/// <returns>The generic Type returned from the operation's execution</returns>
public static T ExecuteTimedAction<T>(string actionText, Func<T> executeFunc, Action<string> logAction)
{
string beginText = string.Format("Begin Execute Timed Action: {0}", actionText);
if (null != logAction)
{
logAction(beginText);
}
else
{
LogUtil.Log(beginText);
}
Stopwatch stopWatch = Stopwatch.StartNew();
T t = executeFunc(); // Execute the action
stopWatch.Stop();
string endText = string.Format("End Execute Timed Action: {0}", actionText);
string durationText = string.Format("Total Execution Time (for {0}): {1}", actionText, stopWatch.Elapsed);
if (null != logAction)
{
logAction(endText);
logAction(durationText);
}
else
{
LogUtil.Log(endText);
LogUtil.Log(durationText);
}
return t;
}
public static void ExecuteTimedAction(string actionText, Action executeAction)
{
bool executed = ExecuteTimedAction<bool>(actionText, () => { executeAction(); return true; }, null);
}
/// <summary>
/// Method for performing an operation and tracking the time it takes to complete (does not return a value)
/// </summary>
/// <param name="actionText">Title for the log entry</param>
/// <param name="action">The action (delegate void) to execute</param>
public static void ExecuteTimedAction(string actionText, Action executeAction, Action<string> logAction)
{
bool executed = ExecuteTimedAction<bool>(actionText, () => { executeAction(); return true; }, logAction);
}
#endregion
Then the Log function looks something like this, as you can see my log function is not hardcoded into the ExecuteTimedAction, so I can pass any log action to it.
In the log class I save the Entry assembly name once in a static variable and use it for all the logs...
private static readonly string _entryAssemblyName = Assembly.GetEntryAssembly().GetName().Name;
Hope this gives you enough food for thought on some refactoring!