8

We all know that the using statement is really good for resources you want to cleanup in a timely manner, such as an open file or database connection.

I was wondering if it would be considered a good thing to use the statement in cases where resources cleanup isn't the goal of the Dispose() method, but rather resetting to a previous state.

For example, a class that allows a using statement to wrap a procedure that takes a noticeable amount of time and changes the Cursor to a waiting state.

class CursorHelper : IDisposable
{
   readonly Cursor _previousState;
   public CursorHelper(Cursor newState)
   {
      _previousState = Cursor.Current;
      Cursor.Current = newState;
   }

   public void Dispose()
   {
      Cursor.Current = _previousState;
   }
}

Then the class can be used as such, without worrying about reverting the Cursor when your are done.

public void TimeIntensiveMethod()
{
   using (CursorHelper ch = new CursorHelper(Cursors.WaitCursor))
   {
      // something that takes a long time to complete
   }
}

Is this an appropriate use of the using statement?

Cemafor
  • 1,633
  • 12
  • 27
  • 1
    I don't think what you have posted is anything wrong, however I think by implementing an interface for something as simple as your example is overkill and misleading. – JonH May 29 '13 at 13:36
  • 1
    My understanding is that `using` calls `IDisposable.Dispose ()` at the end of its scope. I see nothing wrong with this use of `using`, but could it be a "bad" usage of `IDisposable`? – Mathieu Guindon May 29 '13 at 13:38
  • 3
    http://bytes.com/topic/net/answers/568992-idisposable-beyond-memory-management – I4V May 29 '13 at 13:48
  • @I4V nice read, thanks for posting! – Mathieu Guindon May 29 '13 at 16:30

5 Answers5

5

In reality using is simply syntactical sugar for try/finally, so why don't you just do plain old try/finally like below...

try
{
    // do some work
}
finally
{
    // reset to some previous state
}

imho implementing the Dispose method to reset to some state would be very misleading, especially if you have consumers for your code.

Jason
  • 3,844
  • 1
  • 21
  • 40
  • Not sure I agree with that. `using` statements remove some litter code that isn't describing anything useful, plus you can stack them. They compile into something a little more than a `try/finally` but in essence this is what is boils down to. I use them for actions that require follow up actions. Rx does this for certain things in they message pushing and it can work quite well. At the end of the day, this sort of thing is entirely opinionated, but otherwise incurs no harm if done one way or the other. – Adam Houldsworth May 29 '13 at 13:56
  • @AdamHouldsworth Yes it boils down to preference, and in the end `using` doesn't provide you much more than a `try/finally` I'd be happy to be enlightened otherwise. imho stacking usings gives you nothing especially, one can (though would be disgusting) stack try/finally's, or use one try/finally to execute all the Dispose methods in one finally block. I agree that `Dispose` is just a method and a name and you can do as you see fit, but it's like saying *die* and *sleep* are just words, we could all agree to switch their usage, and say `john fell to death` when he went to sleep :) – Jason May 29 '13 at 14:02
  • Fortunately with `using` you don't actually see `Dispose`. It just saves quite a few lines of superfluous code, the stacking just saves a few brackets and a few more lines. You can even comma-delimit the items inside a single using if they are the same type. All of this for me boils down to visually how the code looks - if you take that out of it then yes, `using` over `try-finally` gives you nothing in *this* instance. However, `using` is not translated into just a simple `try-finally` so it isn't fair to say it gives you nothing in all cases. – Adam Houldsworth May 29 '13 at 14:06
  • @Jason - "in the end using doesn't provide you much more than a try/finally" - the increased conciseness is obviously valuable, or the using statement wouldn't have been introduced in the first place. The question is whether or not one should deviate from the original purpose of the using statement, to which my answer is: it depends... – Joe May 29 '13 at 14:10
  • @AdamHouldsworth I'm still learning, and would sure like to know what it really translates into, according to MSFT, http://msdn.microsoft.com/en-us/library/yh598w02.aspx, `using` gets translated by the compiler to a `try/finally` :) – Jason May 29 '13 at 14:11
  • @Jason It gets translated into a nested `try-finally` and uses the `IDisposable` interface directly. Its semantic effect is `try-finally`, but its implementation is more nuanced. Please see this question and my answer: http://stackoverflow.com/questions/11778790/empty-using-statement-in-dispose/11778831#11778831 – Adam Houldsworth May 29 '13 at 14:14
  • @Joe I am not disputing the fact that it comes down to preference, I'm no purist by any means, but I do believe if you provide me a method called `Dispose` it should do some sort of clean up, not reset to a particular state. And I stand by that statement, I have yet to see what else it provides, aside from a few lines of "prettier" code, remember beauty is in the eye of the beholder ;) – Jason May 29 '13 at 14:17
  • @AdamHouldsworth great answer btw I upvoted it. I forgot about the interface and null check parts, not to be stubborn, I do think in this case the op should not abuse the using/dispose – Jason May 29 '13 at 14:36
  • @Jason I can see the reasons for the "abuse" label, but I'll shamefully admit I've done this exact use case in our code more than once >.< As I get older, I'm becoming more pragmatic about code style and what I choose to do. I'm no purist, but the largest two reasons I take for style choices are 1) expressing intent and 2) readable code. If I saw this code again I'd debate re-writing it. – Adam Houldsworth May 29 '13 at 14:36
  • @AdamHouldsworth shame on you, you should be hung for your sins :P j/k – Jason May 29 '13 at 14:37
5

There are certainly precedents for (ab)using the using statement in this way, for example FormExtensions.BeginForm in the ASP.NET MVC framework. This renders a <form> closing tag when it's disposed, and it's main purpose is to enable a more terse syntax in MVC views. The Dispose method attempts to render the closing tag even if an exception is thrown, which is slightly odd: if an exception is thrown while rendering a form, you probably don't want to attempt to render the end tag.

Another example is the (now deprecated) NDC.Push method in the log4net framework, which returns an IDisposable whose purpose is to pop the context.

Some purists would say it's an abuse, I suggest you form your own judgement on a case-by-case basis. Personally I don't see anything wrong with your example for rendering an hourglass cursor.

The discussion linked in a comment by @I4V has some interesting opinions - including arguments against this type of "abuse" from the ubiquitous Jon Skeet.

Joe
  • 122,218
  • 32
  • 205
  • 338
  • 1
    I agree with Jon's statement to avoid using it in publicly exposed items whereas it might be used internally, that tends to be our balance. – Adam Houldsworth May 29 '13 at 14:09
  • @AdamHouldsworth, I also have sympathy with some of Jon Skeet's comments in the linked discussion. But now that such abuses are to be found in mainstream libraries such as ASP.NET MVC, I can't help feeling that the genie's already out of the bottle. – Joe May 29 '13 at 14:15
  • Indeed, but that urge to have pretty looking code versus slightly uglier but functionally sound code gets worse as the days go by in .NET, LINQ and extension methods touting the fluent style are a killer for me, it's a sheer force of will to *not* code like that at times. Who knows, this could be a social indicator on what languages developers used to use prior to creating .NET libraries. My personal background is very limited so C# is predominantly my main language by a long way. Language experiences impact coding styles, as does peer influence, and I see this as a style problem. – Adam Houldsworth May 29 '13 at 14:19
4

I am opposed to this and believe it to be an abuse. I also think that RAII in C++ is a terrible idea. I am aware that I am in a minority on both positions.

This question is a duplicate. For detailed reasons on why I think this is an unwarranted abuse of the using statement, see: https://stackoverflow.com/a/2103158/88656

Community
  • 1
  • 1
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
2

No, it isn't appropriate use of using and/or Dispose. The pattern has a very clear use ("Defines a method to release allocated resources."), and this isn't it. Any future developers using this code would look upon it with the contempt warranted for such evil.

If you want this kind of behaviour then implement events and expose them, calling code can subscribe to them and manage the cursor, if needs be, otherwise the cursor should be manageable by general parameters and maybe using Begin and End methods (although such naming conventions are generally reserved for asynchronous implementations of methods, you get the picture) - hacking this way doesn't actually buy you anything.

Grant Thomas
  • 44,454
  • 10
  • 85
  • 129
-1

I my opinion it makes sense to use using-Disposable thing in a way other than just disposing objects. Of course it depends on the context and usage. If leads to a more readable code then it is ok.

I have had used it in a unit of work & repository pattern implementation like:

public class UnitOfWork: IDisposable  {
    // this is thread-safe in actual implementation
    private static Stack<UnitOfWork> _uowStack = new Stack<UnitOfWork>();
    public static UnitOfWork Current {get { return _uowStack.Peek(); }} 

    public UnitOfWork() {
        _uowStack.Push(this);
    }

    public void Dispose() {
        _ouwStack.Pop();
    }

    public void SaveChanges() {
        // do some db operations
    }
}

public class Repository {
    public void DoSomething(Entity entity) {
        // Do Some db operations         

        UnitOfWork.Current.SaveChanges();
    }
}

With this implementation it is guaranteed that nested operations will use their corresponding UnitOfWork without passing parameter. Usage is like.

using (new UnitOfWork()) 
{
    var repo1 = new UserRepository();
    // Do some user operation

    using (new UnitOfWork())  
    {
         var repo2 = new AccountingRepository();
         // do some accounting
    }

    var repo3 = new CrmRepository();
    // do some crm operations
}

In this sample repo1 and repo3 use the same UnitOfWork while repo2 uses a different repository. What reader reads is "using new unit of work" and it makes a lot of sense.

Mehmet Ataş
  • 11,081
  • 6
  • 51
  • 78