CODEWITHSUNDEEP
c#.nettimeout
John Ruiz
John Ruiz

Reputation: 2391

API Design for Timeouts: TimeoutException or boolean return with out parameter?

The scenario is RPC over message queues - since the underlying mechanism is asynchronous, clients should specify how long they want to wait for a response before timing out. As the client, which of these two code snippets would you rather use?

Most importantly: as a user of the GetResponseTo() method, why would you prefer one over the other? How does your choice make your code more extensible, more readable, more testable, etc?

try
{
    IEvent response = _eventMgr.GetResponseTo(myRequest, myTimeSpan);
    // I have my response!
}
catch(TimeoutException te)
{
    // I didn't get a response to 'myRequest' within 'myTimeSpan'
}

OR

IEvent myResponse = null;

if (_eventMgr.GetResponseTo(myRequest, myTimeSpan, out myResponse)
{
    // I got a response!
}
else
{
    // I didn't get a response... :(
}

For your information, here's the current implementation of GetResponseTo():

public IEvent GetResponseTo(IEvent request, TimeSpan timeout)
{
    if (null == request) { throw new ArgumentNullException("request"); }

    // create an interceptor for the request
    IEventInterceptor interceptor = new EventInterceptor(request, timeout);

    // tell the dispatcher to watch for a response to this request
    _eventDispatcher.AddInterceptor(interceptor);

    // send the request
    _queueManager.SendRequest(request);

    // block this thread while we wait for a response.  If the timeout elapses,
    // this will throw a TimeoutException
    interceptor.WaitForResponse();

    // return the intercepted response
    return interceptor.Response;
}

Upvotes: 2

Views: 346

Answers (5)

John Ruiz
John Ruiz

Reputation: 2391

I have elected to use the out parameter.

I wanted to mark someone else as the answer, but I am not able to do so. I attempted to implement the TPL-based approach, but was unable to do so, based on the question/answer that I linked in my comments.

I do not want to muddy my event model by introducing even more concepts, as @sll suggests.

And even though @dasheddot prefers the exception Version, @sll has a good point that someone trying to send a bunch of requests and get a bunch of responses in a loop might have to deal with a lot of exceptions.

// potentially 10 exceptions?  meh... let's not go down this road.
for(int i=0;i<10;i++)
{
  try
  {
     IEvent response = _eventMgr.GetResponseTo(myRequest, myTimeSpan);

     // I have my response!
  }
  catch(TimeoutException te)
  {
     // I didn't get a response to 'myRequest' within 'myTimeSpan'
  } 
}

Upvotes: 0

sll
sll

Reputation: 62564

Exceptions are heavy and messy, each API method call should be wrapped by try/catch/finally to hanle custom exception. This approach is not developer-friendly so I do not like it.

Considering that GetResponse() call itself is synchronous for API consumer - it is pretty normal to return a value of operation, but I would suggest introducing something more abstract and informative rather than simple bool state, so you can return any state provided by the underlying messaging system, this could be a custom error code, message, or even object. So since this is API - put interface as well:

enum OperationStatus
{
   Unknown,
   Timeout,
   Ok
}

// pretty simple, only message and status code
interface IOperationResult<T>
{
      OperationStatus Status { get; }
      string Message { get; }
      T Item { get; }      
}


class GetResponseResult : IOperationResult<IEvent>
{
   ...
} 

class EventManager
{
     public IOperationResult<IEvent> GetResponseTo(
                                      IRequest request, 
                                      TimeSpan timeInterval)
    {    
        GetResponseResult result;  

        // wait for async request                 
        // ...

        if (timeout)
        {
          result = new GetResponseResult 
                        { 
                           Status = OperationStatus.Timeout,
                           Message = underlyingMessagingLib.ErrorMessage
                        };
        }
        else
        {
          result = new GetResponseResult 
                        { 
                           Status = OperationStatus.Ok,
                           Item = response
                        };
        }

        return result;
    }
}

Upvotes: 0

dasheddot
dasheddot

Reputation: 2966

Personally i would prefer the exception Version. If i specify some timeout my opinion is that this IS a exception then if i couldn't get a result within the specified timespan. I don't think event based notification is the best decision here. The following Logic depends on the result so it doesn't make Sense for me. But if you want to provide asynchronous Methods too, the Task thing is a good idea like stated by dtb

Upvotes: 0

RockThunder
RockThunder

Reputation: 146

You could look to use AutoResetEvent class this will handle the plumbing for second one.

Try to avoid your first code snippet as exceptions are expensive

Upvotes: 1

dtb
dtb

Reputation: 217411

Neither first nor second, I would like to use the Task Parallel Library, which is the recommended way of doing all things asynchronous beginning with .NET 4.5:

Task<IEvent> task = _eventMgr.GetResponseToAsync(myRequest);

if (task.Wait(myTimeSpan))
{
    // I got a response!
}
else
{
    // I didn't get a response... :(
}

Upvotes: 1

Related Questions