Search code examples
c#business-logic

Enforcing an "end" call whenever there is a corresponding "start" call


Let's say I want to enforce a rule:

Everytime you call "StartJumping()" in your function, you must call "EndJumping()" before you return.

When a developer is writing their code, they may simply forget to call EndSomething - so I want to make it easy to remember.

I can think of only one way to do this: and it abuses the "using" keyword:

class Jumper : IDisposable {
    public Jumper() {   Jumper.StartJumping(); }
    public void Dispose() {  Jumper.EndJumping(); }

    public static void StartJumping() {...}
    public static void EndJumping() {...}
}

public bool SomeFunction() {
    // do some stuff

    // start jumping...
    using(new Jumper()) {
        // do more stuff
        // while jumping

    }  // end jumping
}

Is there a better way to do this?


Solution

  • I'm going to disagree with Eric: when to do this or not depends on the circumstances. At one point, I was reworking my a large code base to include acquire/release semantics around all accesses to a custom image class. Images were originally allocated in unmoving blocks of memory, but we now had the ability to put the images into blocks that were allowed to be moved if they hadn't been acquired. In my code, it is a serious bug for a block of memory to have slipped past unlocked.

    Therefore, it is vital to enforce this. I created this class:

    public class ResourceReleaser<T> : IDisposable
    {
        private Action<T> _action;
        private bool _disposed;
        private T _val;
    
        public ResourceReleaser(T val, Action<T> action)
        {
            if (action == null)
                throw new ArgumentNullException("action");
            _action = action;
            _val = val;
        }
    
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
    
        ~ResourceReleaser()
        {
            Dispose(false);
        }
    
        protected virtual void Dispose(bool disposing)
        {
            if (_disposed)
                return;
    
            if (disposing)
            {
                _disposed = true;
                _action(_val);
            }
        }
    }
    

    which allows me to do make this subclass:

    public class PixelMemoryLocker : ResourceReleaser<PixelMemory>
    {
        public PixelMemoryLocker(PixelMemory mem)
            : base(mem,
            (pm =>
                {
                    if (pm != null)
                        pm.Unlock();
                }
            ))
        {
            if (mem != null)
                mem.Lock();
        }
    
        public PixelMemoryLocker(AtalaImage image)
            : this(image == null ? null : image.PixelMemory)
        {
        }
    }
    

    Which in turn lets me write this code:

    using (var locker = new PixelMemoryLocker(image)) {
        // .. pixel memory is now locked and ready to work with
    }
    

    This does the work I need and a quick search tells me I needed it in 186 places that I can guarantee won't ever fail to unlock. And I have to be able to make this guarantee - to do otherwise could freeze a huge chunk of memory in my client's heap. I can't do that.

    However, in another case where I do work handling encryption of PDF documents, all strings and streams are encrypted in PDF dictionaries except when they're not. Really. There are a tiny number of edge cases wherein it is incorrect to encrypt or decrypt the dictionaries so while streaming out an object, I do this:

    if (context.IsEncrypting)
    {
        crypt = context.Encryption;
        if (!ShouldBeEncrypted(crypt))
        {
            context.SuspendEncryption();
            suspendedEncryption = true;
        }
    }
    // ... more code ...
    if (suspendedEncryption)
    {
        context.ResumeEncryption();
    }
    

    so why did I choose this over the RAII approach? Well, any exception that happens in the ... more code ... means that you are dead in the water. There is no recovery. There can be no recovery. You have to start over from the very beginning and the context object needs to be reconstructed, so it's state is hosed anyway. And by comparison, I only had to do this code 4 times - the possibility for error is way, way less than in the memory locking code, and if I forget one in the future, the generated document is going to be broken immediately (fail fast).

    So pick RAII when you absolutely positively HAVE to have the bracketed call and can't fail. Don't bother with RAII if it is trivial to do otherwise.