RouteLinker shouldn't dispose of the HttpRequestMessage it gets via the constructor

[dhilgarth]

The RouteLinker class doesn't own the request and is not responsible for disposing it - that's the job of the WebApi infrastructure.
I think it is actually a bug to dispose of a resource that is owned by someone else.

[ploeh]

That's actually one of those tricky issues that demonstrates just how troublesome the Dispose pattern can be. Basically, there's no clear answer here. No matter what stance one takes, it will be the result of implementation details leaking out.

It's true that in the case of the Web API, the infrastructure will ensure that the HttpRequestMessage instance is disposed of when the request has been handled.

However, the HttpRequestMessage actually isn't defined by the Web API - it's a BCL type. Conceivably, someone might come along and decide to use RouteLinker in a way we haven't thought of yet, but in a way where this disposal doesn't happen automatically. In a situation like that, I consider it more honest to provide a guarantee that since RouteLinker holds a reference to a disposable instance, it will dispose of that instance when disposed of itself.

In other words, we don't know whether the instance is always owned by someone else. In the Web API it probably is, but it may not be.

I totally understand why you consider this a bug, but keep in mind that your perspective is influenced by your assumption that the HttpRequestMessage instance is always going to be managed by the Web API. Not an unfair assumption, but an assumption none the less.

As a counter-argument, I have this to say: I've actually looked at the implementation of HttpRequestMessage.Dispose, and it turns out that it's idempotent. Thus, it's not going to be a problem if it's disposed of multiple times.

Now, that argument is also based on some assumptions, must principal of which is that I understand how HttpRequestMessage.Dispose is implemented, and that I assume it will stay idempotent in the future too.

My argument isn't better than yours, but I also think that it isn't worse either. However, since I prefer to consider each class as a self-contained unit, I prefer the current implementation because it's more independent of its environment.

[dhilgarth]

Thanks for your very elaborate answer.
Your answer centers around the fact that the request message isn't necessarily coming from the Web API. That's a valid point and I agree with it, but actually, that wasn't the point of this issue.
The point was, that whoever created that instance of HttpRequestMessage is the owner of it and not the RouteLinker. And the owner - the one that created the instance - is responsible for disposing of it.
I would rather not implement IDisposable if it can be avoided - and in this case it can, as RouteLinker doesn't create any disposable instances.

Another hint in the direction of my argumentation is that Code Analysis only tells you to implement IDisposable when you create disposable instances, but is does not warn when you get them passed in via a constructor.

[ploeh]

OK, you've convinced me :)

Let's remove the dispose stuff. Will you have the 'honor'?

[dhilgarth]

I most certainly will, thank you! :-)