Should I separate Dispose logic into a partial class file?

Question

While refactoring some C# classes, I've run into classes that implement IDisposable.

Without thinking, I have created partial class files for each class that implements IDisposable interface.

E.g.) For Stamper.cs -> Stamper.cs + Stamper.Dispose.cs where Stamper.cs contains actual logic for stamping and Stamper.Dispose.cs that contains dispose logic

// Stamper.cs
public partial class Stamper
{
// actual logic
}

// Stamper.Dispose.cs
public partial class Stamper: IDisposable
{
// Implement IDisposable
}

When I looked at the code, Stamper.cs now looks a lot cleaner and readable (now about 52 lines instead of 100 lines where around 50 lines was simply a clean-up dispose code)

Am I going too far with this?

*EDIT: Thanks folks for your opinions - I have decided to put two files together into one. The Problem I had faced was that I was actually forgetting to update IDisposable implementation after updating actual logic.

Moreover there wasn't much problem navigating between method in the source code. The first reason seems more than a reason enough to stick with one file solution in my specific case.

Answer 1

It seems about as arbitrary as creating a partial class for constructor logic. Now I have to look at two files to grock that class. Partial classes are only really worth it for designer stuff...

Answer 2

Yes, too far. Whats wrong with just sticking a #Region around the code and folding it so you cant see it?

Answer 3

I would prefer to see the dispose logic in the same file as the resources that warrant implementing IDisposable. Whilst there's an element of subjectivity, I'd say it's too far

Answer 4

I think your solution is unsound. Partial classes should usually only be used to separate developer code from generator code. Regions usually do a better job of adding structure to your code.

Answer 5

If your clean up procedure is heavy, it's acceptable, but not ideal.

It might be a good habit for boiler plate such as exposed events, heavy serialization methods and in your case memory management.

I prefer partial classes than outlines (#region). If you have to use partial class or code outlining to make your code readable, this is usually a sign that the code needs to be changed. Tear the class appart and only as a last resort use partial class (or region) if the code is absolutly necessary for upkeeping that class.

In your case, you could use a class that thinly wraps the unmanaged resource and expose a single Dispose. Then in your other class, use the managed object and Dispose it with no logic.

If your class is only a thin wrap, then I'd say your method is overkill since the whole point of the class is to dispose an unmanaged resource.

Answer 6

Kind of an odd question since it only has an impact on the developer, making it completely up to personal preference. I can only tell you what I would prefer and that would be that I would do it if a significant amount of logic was in the dispose portion.

Answer 7

Personally I try to keep my instantiation/initialization logic and my cleanup/disposal logic side-by-side, it's a good reminder.

As for partial classes, the only time I use them is if a class is very large and can be categorized into groups of methods. Hiding designer code is great too.

Answer 8

I'd favor using a partial class when, and only when, the code in question was computer-generated. If you have many classes which share similar code (which for various reasons has to be repeated, rather than being pulled out into its own class) it may be useful to have a few templates and a program to generate the code based upon such templates. Under that scenario, the templates would be regarded as source files and then generated files as intermediate object-ish code. It would seem entirely appropriate to pull out the template-generated code into partial classes.

In vb.net, such an approach might be nice to allow field declaration, initialization, and cleanup to be safely handled together within an IDisposable object. A moderate amount of boilerplate code is required, but field declarations after that are pretty clean. For example:

' Assuming Option Implicit on:
Dim MyThingie = RegDisposable(New DisposableThingie)
' If Implicit wasn't on:
Dim MyThingie As DisposableThingie = RegDisposable(New DisposableThingie)

RegDisposable would be a class member that would add the new DisposableThingie to a list held by the class. The class' Dispose routine would then Dispose all the items in the list.

Unfortunately, there's no clean way to do anything similar in C#, since field initializers cannot make use of the object about to be constructed (in vb.net, field initializers run after the base object is constructed).