1

I am maintaining code that was written by another developer. I came across this in a rather large class (assume 20 or 30 functions--I show only 3 below):

public class Widget
{
     DisposableObject do = new DisposableObject();

     private void function1()
     {
          do.something();
          ...
     }

     private void function2()
     {
          do.something();
          ...
     }

     private void function3()
     {
          do.something();
          ...
     }

}

For my question, what 's important to consider the following:

  • DisposableObject implements IDisposable and therefore can be used with the C# using statement.
  • do is a class-global object that's being used by more than one function within the class. Assume for the sake of discussion that I cannot easily determine if one function does something with the do object that changes its state and then calls another function within the class. Also, no function within the class passes do as an argument--they all tap into the class-global instance.
  • I have detected that object do holds a connection to a database because some of these functions are buggy and the object isn't getting disposed of properly.

Ideally, I want to somehow refactor this class so that I can do something like this:

using (DisposableObject do = new DisposableObject())
{ ... }

Unfortunately, I don't know enough about how the do object is changing among this classes various functions--otherwise I could just remove the global declaration and add using statements to each function.

Is there a neat way to use, say, dependency injection or perhaps a factory to ensure that this class continues to function in its dysfunctional way yet would properly dispose of object do when the code that consumes this class is finished doing what it needed to do? I want to ensure that do is properly disposed of but I also want to be sure that I don't break the existing code.

Thank you for your help.

Jazimov
  • 12,626
  • 9
  • 52
  • 59
  • 4
    The best advise I can give you is first try to logically break this class down into related groups of methods. Then, for each of these create tests that ensure the functionality is working. Then slowly start breaking those by disposing the object/refactoring the code. – Yuval Itzchakov Nov 21 '15 at 12:34
  • 1
    Maybe I miss some point - but why don't you make the Widget class itself IDisposable & dispose the do object once the Widget Dispose method is called ? – Ondrej Svejdar Nov 21 '15 at 13:51
  • @Ondrej: No, you didn't miss a thing--I think that's exactly the kind of suggestion I needed--I was too focused on solving the problem intrinsically rather than extrinsically! So after I make Widget inherit IDisposable, what else would I need to do? Do I need a Widget destructor or do I just need to override a Dispose method? – Jazimov Nov 21 '15 at 18:15
  • _"what else would I need to do"_ -- if you are only disposing managed objects, then you only need to implement the `IDisposable` interface, i.e. the `Dispose()` method. The finalizer (aka "destructor"...I prefer to not use "destructor" , because C# finalizers don't work like C++ destructors, and using the same word can be confusing) is needed only if your class is hold _unmanaged_ resources (and even then, it's better to use `SafeHandle` instead of implementing a finalizer). – Peter Duniho Nov 22 '15 at 00:45
  • See also [Should IDisposable be applied cascadingly?](https://stackoverflow.com/q/1461557). There is already lots of advice on when and how to implement `IDisposable`. If you have some _specific_ problem doing so, please post a new question that includes [a good, _minimal_, _complete_ code example](http://stackoverflow.com/help/mcve) that reliably reproduces that specific problem, with a precise explanation of what that code does and how that's different from what you want it to do. – Peter Duniho Nov 22 '15 at 00:51
  • Thank you, and I got it. But I do not think my original question was a duplicate--questions like it were asked but not specifically as I asked (which was more about how to refactor a global object that needed to be destroyed after being used by multiple class-level functions). What is a duplicate is my follow-up question about implementing IDisposable, which really wasn't the thrust of my original message... – Jazimov Nov 22 '15 at 04:24

0 Answers0