EPiServer.Logging beta messageFormatter arguments

tss
tss
Vote:
 

Hi

I have a suggestion to the new EPiServer.Logging beta

The new ILogger.Log method has the following signature:

void Log(Level level, TState state, TException exception, Func messageFormatter) where TException : Exception;

Wouldn't it be better to have a simple signature like the following:

void Log(Level level, TState state, TException exception, Func messageFormatter) where TException : Exception;


This would make it simpler to use and TState and TException is still accessible where the Log method is called:

_logger.Log(Level.Error, (object) null, exception, () => { return string.Format("Sample Format Message {1}", exception.Message) });



#115662
Jan 15, 2015 15:21
Vote:
 

I would go with overload. Sometimes it's necessary to know what kind of exception was thrown or at what level it was.

#115675
Jan 16, 2015 0:53
tss
Vote:
 

You still have the type of the exception inside the scope of the messageFormat its just wrapped earlier then the Log method

But if they go for overloads they should also make some that dont require the TState object as that can be a bit confusing.

#115698
Jan 16, 2015 11:36
Vote:
 

How can I access type of the exception if formatter would be another method? Like:

public string FormatErrorMessage() { .. }

Log(Error, null, exc, FormatErrorMessage);
#115700
Jan 16, 2015 11:48
tss
Vote:
 
I would do it like:
public string FormatErrorMessage(FormatException ex) { .. }
 
Log(Error, null, exc, () => FormatErrorMessage(exc));
#115702
Edited, Jan 16, 2015 11:49
Vote:
 

right, you can pass it through lambda :)

#115703
Jan 16, 2015 11:53
tss
Vote:
 

I just think they could make a simpler interface this way, and avoid the whole idea about the TState object in most cases,

And we could avoid building boilerplate code around the currently verbose Log signature.

#115704
Jan 16, 2015 11:56
* You are NOT allowed to include any hyperlinks in the post because your account hasn't associated to your company. User profile should be updated.