Is abusing IDisposable to benefit from "using" statements considered harmful?

Question

The purpose of the interface IDisposable is to release unmanaged resources in an orderly fashion. It goes hand in hand with the using keyword that defines a scope after the end of which the resource in question is disposed of.

Because this meachnism is so neat, I've been repeatedly tempted to have classes implement IDisposable to be able to abuse this mechanism in ways it's not intended for. For example, one could implement classes to handle nested contexts like this:

class Context : IDisposable
{
    // Put a new context onto the stack
    public static void PushContext() { ... }

    // Remove the topmost context from the stack
    private static void PopContext() { ... }

    // Retrieve the topmost context
    public static Context CurrentContext { get { ... } }

    // Disposing of a context pops it from the stack
    public void Dispose()
    {
        PopContext();
    }
}

Usage in calling code might look like this:

using (Context.PushContext())
{
   DoContextualStuff(Context.CurrentContext);
} // <-- the context is popped upon leaving the block

(Please note that this is just an example and not to the topic of this question.)

The fact that Dispose() is called upon leaving the scope of the using statement can also be exploited to implement all sorts of things that depend on scope, e.g. timers. This could also be handled by using a try ... finally construct, but in that case the programmer would have to manually call some method (e.g. Context.Pop), which the using construct could do for thon.

This usage of IDisposable does not coincide with its intended purpose as stated in the documentation, yet the temptation persists.

Are there concrete reasons to illustrate that this is a bad idea and dispell my fantasies forever, for example complications with garbage collection, exception handling, etc. Or should I go ahead and indulge myself by abusing this language concept in this way?

There are lot other concepts that could be misused, the above is one of them, the only thing is **readability**, remember your code should be clear/readable and maintainable by other developers. — Habib, Apr 14 '15 at 13:20
I agree with Habib. Always think about the poor bastard that will have to work with your code 18 months from now. there's a good chance that bastard will be you :-) — Zohar Peled, Apr 14 '15 at 13:24
It's a subjective question, but you could note that the ASP.NET MVC framework abuses IDisposable in this way - for example `FormExtensions.BeginForm` — Joe, Apr 14 '15 at 13:24
possible duplicate of [Is it abusive to use IDisposable and "using" as a means for getting "scoped behavior" for exception safety?](http://stackoverflow.com/questions/2101524/is-it-abusive-to-use-idisposable-and-using-as-a-means-for-getting-scoped-beha) — GSerg, Apr 14 '15 at 13:29
I have edited the question make it clearer that I'm not looking for opinions (I already have one: "This works great!"), but for concrete reasons why this usage could/will lead to issues. — waldrumpus, Apr 14 '15 at 14:14
I will also check the linked question to see if mine can be closed as a duplicate. — waldrumpus, Apr 14 '15 at 14:15

spender · Accepted Answer · 2015-04-14T13:27:15.030

10

So in asp.net MVC views, we see the following construct:

using(Html.BeginForm())
{
    //some form elements
}

An abuse? Microsoft says no (indirectly).

If you have a construct that requires something to happen once you're done with it, IDisposable can often work out quite nicely. I've done this more than once.

edited Apr 14 '15 at 13:27

answered Apr 14 '15 at 13:23

spender

117,338
33
229
351

4

Similar with transaction scope. – Magnus Apr 14 '15 at 13:24
1

Agreed with spender. It's used by Microsoft in ways that don't always release resources, so I think it's "allowed." That being said, readability should be your concern. I think the `BeginForm` is clear; Popping from a list, not so much. – Matt Grande Apr 14 '15 at 13:26
If the `using` line contains "Push", I'd say popping is pretty obvious. – Medinoc Apr 14 '15 at 14:05
@Medinoc Sure, just as if it contains `Begin` I'd say `End` is pretty obvious... But in both cases, easy to forget. The using construct simply increases your chance of falling into the pit of success. – spender Apr 14 '15 at 14:10
1

Why not just use a `try/finally`? It's just as readable. – Chris Dunaway Apr 14 '15 at 15:16
2

@ChrisDunaway `using` is already syntactic sugar around `try/finally`, but with a handy standard interface. Without `IDisposable`, the caller needs to know how to clean-up, whereas `using` puts the burden on the creator. – Andrew Hanlon Apr 14 '15 at 18:49

score 3 · Answer 2 · answered Apr 14 '15 at 13:28

3

"Is it an abuse of the IDisposable interface to use it this way"? Probably.

Does using using as a purely "scoping" construct make for more obvious intent and better readability of code? Certainly.

The latter trumps the former for me, so I say use it.

answered Apr 14 '15 at 13:28

Ian Kemp

28,293
19
112
138

Why not just use `try/finally`? It's just as readable. – Chris Dunaway Apr 14 '15 at 15:17
@ChrisDunaway in this scenario I'd disagree, mainly because (a) in this case you'd end up with an empty `finally` and (b) IMO because `using` declares scoped intent far more clearly than `try/finally`. To me, `using` says "I am doing something with this object and want it to go away afterwards"; `try/finally` says "I may get an exception here and I want to handle it". – Ian Kemp Apr 14 '15 at 15:33
I would have to disagree with part of your statement. `try/finally` (with no catch clause) does not imply anything about exceptions. The finally would have whatever necessary cleanup code and says "I want to do the follow and and then finally clean up". At least that's how I would read it. – Chris Dunaway Apr 14 '15 at 15:38

score 0 · Answer 3 · answered Apr 14 '15 at 13:32

0

You certainly wouldn't be the first one to 'abuse' IDisposable in that way. Probably my favorite use of it is in timers, as the StatsD.NET client demonstrates:

using StatsdClient;
...
using (statsd.LogTiming( "site.db.fetchReport" ))
{
  // do some work
}
// At this point your latency has been sent to the server

In fact, I'm pretty sure Microsoft themselves use it in some libraries. My rule of thumb would be - if it improves readbility, go for it.

answered Apr 14 '15 at 13:32

georgevanburgh

169
1
8

Why not just use `try/finally`? It's just as readable. – Chris Dunaway Apr 14 '15 at 15:18
@ChrisDunaway The point here is to time the operation within the using block, not protecting against exceptions. You can read it as `using myTimer, do ...` and, by doing so, condense what would otherwise be three lines of code (timer.Start, timer.Stop, timer.SendToServer) into something (arguably) more readable. – georgevanburgh Apr 14 '15 at 16:00

Is abusing IDisposable to benefit from "using" statements considered harmful?

3 Answers3

Linked