EPiServer.Logging beta messageFormatter arguments

November Happy Hour will be moved to Thursday December 5th.

AI OnAI Off

Home / Forums / Developers (Main Products) Forum / CMS 7.5 - 11 /

tss

Vote:

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

valdis

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

valdis

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

valdis

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