1

We have a logging class, that has a log method with params:

public void LogTrace(String message, params object[] parameters) { .. .. String.Format(message, Parameters);

What would be the .NET convention/expectation for what it would do when LogTrace is called without params but using some formatting placeholders? e.g.

LogTrace("This is a {poorly written} log message");

or

LogTrace("This is {0} log message");

Specifically - would you expect this to throw an exception, or to just log the plain text as-is? In the first case it seems "nice" that it just logs plain text, but in the second case it probably indicates you actually forgot a parameter.

Conceptually, it seems like there are two different functions here, one to log plain text, and one that logs a formatted message. So perhaps there should be two separate methods?

Improve this question
3
  • from what i understood you could make just 1 method check the count of params == 0 do something with only the string if not use the parameters. Commented Oct 23, 2013 at 23:24
  • yes, we started out with 2 methods, then changed to 1, but from a design point of view they seem like significantly different functions ... Commented Oct 23, 2013 at 23:30
  • i dont know whats behind but,for example if you know you only pass 2 or 3(could be more)parameters at most,then you could opt for optional parameters. Commented Oct 23, 2013 at 23:36

3 Answers 3

Reset to default
1
  • String.Format("This is a {0} {1} log message", "poorly"); will throw (one parameter too few)
  • This is a good thing: It means, that there was an error in the logging part of whatever facility was involved. This should be logged by the logger.
  • You can still work around this (and produce the original message as good as possible) by try-catch-looping allways passing one parameter (e.g. "[undefined]") more, this will log This is a poorly [undefined] log message. Don't make this an endless loop!
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

This is an interesting concept ; perhaps logging should not throw errors but rather try to do "it's best" to log the data it has. But then again, if there is a problem in the logging, it's a bug like any other and an exception will make sure it's noticed.
I think both should be done: Log the logging problem AND log the original message as a best effort. Exotic errors are often less-than-oerfectly tested and an additional error in the logging should not mask another logworthy condition. I personally think, logging should never throw as long as the logging facility itself is OK.
Good point, we can draw a distinction between the logger saying "I'm broken" which is really serious and needs to be made visibile (exception), vs "the person using me is broken", in which case I want to be helpful.
Exactly what I ment! "Log disk full" should throw, but "Malformed log message" should not.
1

string.Format will throw an exception when it expects parameters and you do not provide them. It will throw an exception when message is not formatted correctly - with rubbish between brackets. You can guard against empty parameters list

public void LogTrace(String message, params object[] parameters) { if(parameters != null && parameters.Length > 0) var s = String.Format(message, Parameters); else var s = message; // do something with s }

It will still have problems when the number of parameters does not correspond to the message, but I would let it throw an exception rather than trying to mask the problem and logging the obscure message.

Improve this answer

2 Comments

Yes, we do that now. But is that the best thing to do? Is that what a .net programmer would expect? That the particular case of zero parameters is allowed to have invalid formatting instructions?
Personally, I would expect the method to throw an exception if I'm wrong rather than stay ignorant about faulty code, even if it is logging. If you don't trust developers to correctly call the method, may be it could be refactored to minimise the risk of misuse.
1

I think that two methods would be ok (if you REALLY need them):

LogTrace(message, params) { // just don't catch exceptions from String.Format method, so It // is possible to throw FormatException or ArgumentNullException }

and 

LogTrace(message) { // place a plain text, e.g. This is {0} log. }

I think this is easy to understand and easy to use.

  • If you don't have a params dont expect that the message will be formatted in any way.

  • If you use a method with Parameters you have to put there not null object and you should expect that a message and parameters works with each other. In case of error in your code you should expect a standard behaviour as in .NET framework was implemented: http://msdn.microsoft.com/pl-pl/library/system.string.format.aspx#Format_Exceptions

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.