views:

110

answers:

7

What are, in your opinion, the pros/cons of the following extension method?

static class Log
{
    public static string AddToLog(this string input)
    {
        Console.WriteLine(input);
        return input;
    }

    public static string AddToLog(this string input, string format)
    {
        Console.WriteLine(format, input);
        return input;
    }
}

Usage scenario:

class Program
{
    static void Main(string[] args)
    {
        string tableName = "Bills".AddToLog("Default table name is {0}");

        "Starting...".AddToLog();
        "Creating table".AddToLog();
    }
}
+5  A: 

Well they're static for a start, which is going to make testing more difficult and will couple everything more tightly.

I also personally think something like

Logger.Write("Starting..");

is more understandable than

"Starting...".AddToLog();
Grant Crofton
Agreed. Seems like a solution looking for a problem. Not that I'm not guilty of that myself of course :)
Dzejms
Not sure what you mean by static methods making testing more difficult. Personally, there's nothing I love unit testing more than a `public static` method.
ladenedge
Yeah, I'm the same with lambdas.. 'I'm sure I could somehow use a lambda to do this... But how?!'
Grant Crofton
@ladenedge - yeah, I love testing them too! It's the methods which call them that start to get tricky... Unless you use TypeMock or something, but that's cheating!
Grant Crofton
They're only more difficult to test because they use `Console.WriteLine`. `Trace.WriteLine` is more amenable to testing, because you can easily divert the output.
Jordão
@Jordao - But even with something like that, you've got an extra layer in your unit tests than you would have if your logger wasn't static, more setting up to do.
Grant Crofton
@Grant Crofton: oh, I see what you mean now. Yeah, that sucks.
ladenedge
+1  A: 

To me, it isn't very readable.

Just because you can doesn't mean you should :)

John Gardner
+1  A: 

I'm gonna go with not a good idea. I can't think of any reason to do this. It clutters the namespace, and it isn't very intuitive. (At least to me)

EDIT

Now that I think about it, I have seen some advocate extending simple objects like this, but a logger isn't a good case IMHO. If you are providing a .ToX() functionality, like converting an integer to a MPH string or something like that, an extension method might be useful, but a logger is just not a good fit.

Paul
+1  A: 

Normally instead of Console.WriteLine you have a call to your actual logging mechanism

Your logging API (if you are using one) could not only be able to log strings but also log any kind of objects.
For example in log4net you can call .Error method with an object parameter. Then the logging mechanism decides what information about the object to actually log.

The way you have done it defeats that idea.

Carlos Muñoz
+1  A: 

This makes for interesting, ruby like, syntax. However, like John said, just because you can doesn't mean you should.

This would confuse most C# developers out there and add unneeded confusion.

For the the specific purposes of logging there are far better ways to get what you want. My very first question would be, why are you rolling your own logging solution? Logging is a very well solved problem and you shouldn't be wasting dev cycles on something that, log4net for instance, does just fine.

Foovanadil
+1  A: 

For proper logging, you would want far more than just some string. Date, source, category etc., which you may want to store in a more structured manner.

All in all, creating a logging extension method on String feels just plain wrong. The extension method's functionality should have a fairly strong association with the type it's operating upon, in line with the single responsibility principle. In the case as you described, that's clearly being violated.

Wim Hollebrandse
+1  A: 

The biggest problem in this approach is it is nearly impossible to inject dependencies into static/extension methods very cleanly. Meaning your logging solution (presuming it is to become more complex than dumping things to stdout/console/debug) has to be spun up and configured to do just about any sort of testing on the project. Ever.

Wyatt Barnett