Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have the following interface:

public interface ILogger
{
    void Log(string message, string title, StatusType type, DateTime timestamp);
    IAsyncResult LogAsync(string message, string title, StatusType type, DateTime timestamp, AsyncCallback callback);
    void EndLogAsync(IAsyncResult result);
}

Implemented as a file-based logger...

...

    public FileInfo LogFileInfo { get; internal set; }

    public void Log(string message, string title, StatusType type, DateTime timestamp)
    {
        lock (syncObj)
        {
            CheckLogFileSize(); //rotates LogFileInfo through numbered log files

            using (var stream = LogFileInfo.AppendText())
            {
                stream.WriteLine(GetFormattedString(message, title, type, timestamp));
                stream.Flush();
            }
        }
    }

    public IAsyncResult LogAsync(string message, string title, StatusType type, DateTime timestamp, AsyncCallback callback)
    {
        var action = new Action<string, string, StatusType, DateTime>(Log);
        return action.BeginInvoke(message, title, type, timestamp, callback, action);
    }

    public void EndLogAsync(IAsyncResult result)
    {
        ((Action)result.AsyncState).EndInvoke(result);
    }

Notice the AsyncState parameter of BeginInvoke; I pass the delegate to its own invocation, so that I can reference it again on the flip side to end the invocation using the same delegate reference I called BeginInvoke on.

The questions:

  • Is it even necessary to keep the actual invoked delegate reference around, or can I simply new up another reference to that method in the EndLogAsync method?
  • Is this an acceptable use of the AsyncState, given that I have no other use for it?
share|improve this question
3  
Is there any chance you're writing on .NET 4.5? And why not use existing logging solutions (NLog, log4net)? –  almaz Dec 11 '12 at 5:54
    
I think the implementation as it stands is the "most correct" using delegates/BeginInvoke/EndInvoke. Let it ride. –  Jesse C. Slicer Jan 10 '13 at 14:37
    
I really think that LogAsync should return a Task and therefore not accept a callback delegate. The APIs that consist Begin*/End* method pairs are ancient. By using TPL you can also let the implementers of ILogger interface use the new async features in .NET 4.5 more easily. –  Şafak Gür Jan 11 '13 at 14:29

1 Answer 1

To answer your question:

  • you can access the delegate instance by reading ((AsyncResult)asyncResult).AsyncDelegate property (see Executing a Callback Method When an Asynchronous Call Completes section of Calling Synchronous Methods Asynchronously). But it will be easier to cache the instance of the delegate in a readonly field and use it in both methods.
  • yes, you can use AsyncState however you want, and sometimes people use it for passing the delegate being invoked

Note that you're using a separate thread (on a threadpool) each time you write something to log, so there will be contention between multiple threads if you write to log quickly enough. I would recommend using existing logging frameworks like NLog or log4net to avoid inventing a wheel.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.