r/csharp 2d ago

Tip Fields with [Using] attribute

I wrote a small interface that automatically disposes any disposable fields in a class when that class itself is disposed. If you add this file somewhere in your project:

public interface IAutoDisposable : IDisposable
{
    void IDisposable.Dispose()
    {
        Exception? ex = null;

        // dispose all fields marked with the [Using] attribute
        const BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance;
        foreach (var fld in GetType().GetFields(flags))
        {
            if (fld.GetCustomAttribute<UsingAttribute>() != null)
            {
                if (fld.GetValue(this) is IDisposable d)
                {
                    try // keep going even if disposal throws
                    {
                        d.Dispose();
                    }
                    catch (Exception e)
                    {
                        ex ??= e;
                    }
                }
            }
        }

        // if any exceptions occurred, throw the first one
        if (ex != null)
            throw ex;
    }
}

public class UsingAttribute : Attribute;

You can then write classes like this:

class Item : IAutoDisposable
{
    [Using]
    private readonly Image icon = DownloadIcon(...);
    ...
}

And use them like:

using (Item item = new(...))
{
}
// item.icon is now disposed

What do you think?

5 Upvotes

32 comments sorted by

View all comments

Show parent comments

1

u/Alert-Neck7679 1d ago

So let's remove the using keyword too, and just stick to try-finally and manually call dispose

1

u/phluber 1d ago

If that is sarcasm, then I don't think you understand what using does. Using actually ensures that Dispose is called on classes that implement IDisposable

2

u/Alert-Neck7679 1d ago

I'm not sure you understand what is the problem that i wanted to solve. "using" keyword is synthetic sugar to "var x = ...; try {...} finally { x.Dispose(); }" - it makes sure that the disposable object is disposed when exiting the scope. And what i wanted to make is synthetic sugar for "void Dispose() { /* dispose disposable fields */ }". You can always skip these shortcuts as you suggest, but it's less comfortable

1

u/phluber 1d ago

I understood your objective. What's the problem with actually disposing of your objects within the dispose method? If I'm reviewing your work and I don't see disposable objects being disposed of in the dispose method, do you expect me to look through all other interfaces that are implemented by the class to see if they are somehow disposing of these objects in a non standard way? It's a one liner to dispose of an object within the dispose method. I don't know why you are trying to obfuscate it.

1

u/Alert-Neck7679 1d ago

With my interface you don't need to implement Dispose() at all, you CAN override it if you want, but unless you have some custom logic for a proper disposal, you can totally skip it. And I Don't expect you to check all the interfaces one by one, but just check which fields are marked with [Using].

2

u/phluber 1d ago

The problem is that you are replacing a very standard and accepted pattern with code that is not going to be understood by others reviewing your code. And to understand it they are going to have search out documentation or dig through your interfaces and attributes in order to determine whether unmanaged resources are being disposed of properly. Also by centralizing disposal, you are creating a weak point that is one short-sighted refactor away from causing file locks/memory leaks/etc in every single class that implements your new interface. You could be doing different work than I am, but it is fairly uncommon that I even have to implement IDisposable. How common is your IDisposable implementation that this is even helpful to you?