Is a struct to perform cleanup at the end of scope a good C# pattern?

Question

RAII is nice for ensuring you don't fail to call cleanup. Normally, I'd implement with a class. I'm currently using Unity and am conscious of generating garbage in Update (even in editor scripting). I was thinking, is creating a struct wrapper for Begin/End actions a good pattern for avoiding allocation? (Since value types don't allocate on the heap.)

Here's an editor scripting example:

public struct ActionOnDispose : IDisposable
{
    Action m_OnDispose;
    public ActionOnDispose(Action on_dispose)
    {
        m_OnDispose = on_dispose;
    }

    public void Dispose()
    {
        m_OnDispose();
    }
}

EditorGUILayout is a Unity type that exposes several functions that need to be called before and after your code. I'd use ActionOnDispose like this:

public static ActionOnDispose ScrollViewScope(ref Vector2 scroll)
{
    scroll = EditorGUILayout.BeginScrollView(scroll);
    return new ActionOnDispose(EditorGUILayout.EndScrollView);
}

private Vector2 scroll;
public void OnGUI() // called every frame
{
    using (EditorHelper.ScrollViewScope(ref scroll))
    {
        for (int i = 0; i < 1000; ++i)
        {
            GUILayout.Label("Item #"+ i);
        }
    }
}

The above simple example works as expected and Dispose is called once for each call to ScrollViewScope, so it appears correct. Unity even provides a scoped struct: EditorGUI.DisabledScope, but not in many cases. So I wonder if there's a downside to this pattern with structs? (I'd assume if the struct is somehow copied the old copy would be disposed and my end action called twice? I don't see such a scenario, but I'm not very familiar with C# value types.)

Edit: I'm specifically asking if whether it matters that I'm using a struct (a value type). Is abusing IDisposable to benefit from “using” statements considered harmful covers whether this is a good use of IDisposable.

I think it won't be boxed (good): https://stackoverflow.com/questions/2412981/if-my-struct-implements-idisposable-will-it-be-boxed-when-used-in-a-using-statem — idbrii, Jul 12 '18 at 00:23
Maybe the timing of dispose is not reliable (bad)? https://stackoverflow.com/questions/173670/why-is-there-no-raii-in-net — idbrii, Jul 12 '18 at 00:23
Seems fairly straightforward. I recently discovered EditorGui.DisabledScope myself (was using .enabled = true/false prior to that haha). I would change the line to `return new ActionOnDispose (EditorGUILayout.EndScrollView);`, the method group reads easier, though that's a personal preference. Curious, does the Vector2 need to be passed by `ref` at all? — andeart, Jul 12 '18 at 00:40
https://stackoverflow.com/questions/29628384/is-abusing-idisposable-to-benefit-from-using-statements-considered-harmful — spender, Jul 12 '18 at 00:51
@andeart: BeginScrollView uses the vec2 to maintain state between updates. It takes the current scroll region updates it (if something expanded I guess). — idbrii, Jul 16 '18 at 18:01
If you're conscious about generating garbage, why allocate a new `Action`? `try { scroll = EditorGUILayout.BeginScrollView(scroll); } finally { EditorGUILayout.EndScrollView(); }` allocates nothing at all. (You can fix this by caching the delegate for the specific case of `EndScrollView`, but that won't work in general.) — Jeroen Mostert, Jul 16 '18 at 18:23
@JeroenMostert I think `using` provides a simpler and terser method of indicating scope. However, I guess you're saying that assigning the `Action` member of `ActionOnDispose` will generate garbage? (Is that still true if I apply andeart's fix?) Regardless, I could make versions of ActionOnDispose for each of my use cases to avoid use of Action. — idbrii, Jul 16 '18 at 18:37
Assigning does not generate the garbage, writing `new ActionOnDispose(EditorGUILayout.EndScrollView)` does. (Yes, this too allocates a new `Action` under water, it's just syntactic shorthand.) In general, it's very hard to avoid allocation if you're using delegates, but it usually doesn't pay off to do so either. Short-lived objects are collected in gen 0, which is very efficient. My uneducated guess would be that methods like `EditorGUILayout.BeginScrollView` are going to allocate stuff themselves, to the point where one more allocation on your end won't matter much. — Jeroen Mostert, Jul 16 '18 at 18:58
To answer your other question, no, copying the struct would not cause the method to be called twice. The only way to do that is to actually have it wrapped in two different `Dispose` scopes. C# does not have RAII; things going out of scope does nothing. It's the `using` statement that does the actual magic. — Jeroen Mostert, Jul 16 '18 at 19:07
Is there any intention to this code other than avoiding having to call `EditorGUILayout.EndScrollView()` at the end of your code (where the end of your `using` scope is)? How often does this code pattern repeat that you feel the need to hide that one-liner method call? If you really want to encapsulate the `Begin`/`Ènd` methods, why not simply create a class that does so (`Begin` in constructor, `End` in `Dispose`) instead of making an abstract "onDispose" event? — Flater, Jul 18 '18 at 09:16
@Flater that's the goal. [Here's an example of building a character editor UI](https://github.com/okayly/vrgame/blob/f2bde68a309d36cbe48b7bf9ee00a0b7b7c1343c/Assets/G2USample/_2%20Open%20This%20Scene/SampleAssets/Scripts/Database/Editor/SampleCharacters/SampleCharactersEditor.cs). For more complicated panels, you might have nested BeginHorizontal and BeginVertical. I tend to wrap each section in braces. Putting a using on top instead of begin on top and end on bottom seems simpler. — idbrii, Jul 18 '18 at 17:55
@Flater Why do individual classes? Do you have a reason why it's superior? You could do it either way ([someone else did the class version here](https://github.com/dongzhiqiang/Jumper/tree/f727810bb0e8cfd1365a5186c49b9527074e0962/Game/Assets/Common/Scripts/EditorExtend/AutoRelease).)), but what's the benefit? — idbrii, Jul 18 '18 at 17:57
@idbrii I didn't mean individual classes. I simply meant not hiding the end call in a disposing method that isn't visible. You're trading mildly nicer syntax in return for not knowing if and when your end method will be called, on top of the readability issue with hiding the end call and, in extreme cases, encountering resource locking issues or race conditions. — Flater, Jul 18 '18 at 18:32
@Flater My comment about individual classes was a response to "why not simply create a class that does so (Begin in constructor, End in Dispose)". — idbrii, Jul 18 '18 at 21:00
@idbrii But that is only one class: it calls start in the constructor, and the same class calls end in the dispose method. I'm not sure which individual classes you're referring to, there's only one. — Flater, Jul 18 '18 at 21:05

score 0 · Answer 1 · answered Aug 05 '21 at 17:21

Jeroen Mostert's comments as an answer:

Assigning does not generate the garbage, writing new ActionOnDispose(EditorGUILayout.EndScrollView) does. (Yes, this too allocates a new Action under water, it's just syntactic shorthand.) In general, it's very hard to avoid allocation if you're using delegates, but it usually doesn't pay off to do so either. Short-lived objects are collected in gen 0, which is very efficient. My uneducated guess would be that methods like EditorGUILayout.BeginScrollView are going to allocate stuff themselves, to the point where one more allocation on your end won't matter much.

To answer your other question, no, copying the struct would not cause the method to be called twice. The only way to do that is to actually have it wrapped in two different Dispose scopes. C# does not have RAII; things going out of scope does nothing. It's the using statement that does the actual magic.

My conclusion:

I could make versions of ActionOnDispose for each of my use cases to avoid use of garbage-generating Action.

Is a struct to perform cleanup at the end of scope a good C# pattern?

1 Answers1