Skip to main content
added 187 characters in body
Source Link
t3chb0t
  • 44.3k
  • 9
  • 84
  • 190

The TryLock class shouldn't be netested.

I'd also add a factory method to it and make it much simpler like this and create an instance only when a lock could be accuired:

private class TryLockLocker : IDisposable
{
    private object _locked;

    private TryLockLocker(object obj)
    {       
        _locked = obj;
    }

    public static TryLockLocker Create(object obj)
    {
        return Monitor.TryEnter(obj) ? new TryLockLocker(obj) : null;
    }

    public void Dispose()
    {
        Monitor.Exit(_locked);
    }
}

Later instead of checking the property you check if the instance isn't null:

using (var tryLock = TryLockLocker.Create(_executeLock))
{
    if (tryLock == null)
    {
        return;
    }

    ...
}

@variable

This kind of naming can and should really be avoided. I'm sure you can come up with a better name then just a delegate which says pretty nothing about the method.

The TryLock class shouldn't be netested.

I'd also add a factory method to it and make it much simpler like this and create an instance only when a lock could be accuired:

private class TryLock : IDisposable
{
    private object _locked;

    private TryLock(object obj)
    {       
        _locked = obj;
    }

    public static TryLock Create(object obj)
    {
        return Monitor.TryEnter(obj) ? new TryLock(obj) : null;
    }

    public void Dispose()
    {
        Monitor.Exit(_locked);
    }
}

Later instead of checking the property you check if the instance isn't null:

using (var tryLock = TryLock.Create(_executeLock))
{
    if (tryLock == null)
    {
        return;
    }

    ...
}

The TryLock class shouldn't be netested.

I'd also add a factory method to it and make it much simpler like this and create an instance only when a lock could be accuired:

private class Locker : IDisposable
{
    private object _locked;

    private Locker(object obj)
    {       
        _locked = obj;
    }

    public static Locker Create(object obj)
    {
        return Monitor.TryEnter(obj) ? new Locker(obj) : null;
    }

    public void Dispose()
    {
        Monitor.Exit(_locked);
    }
}

Later instead of checking the property you check if the instance isn't null:

using (var tryLock = Locker.Create(_executeLock))
{
    if (tryLock == null)
    {
        return;
    }

    ...
}

@variable

This kind of naming can and should really be avoided. I'm sure you can come up with a better name then just a delegate which says pretty nothing about the method.

Source Link
t3chb0t
  • 44.3k
  • 9
  • 84
  • 190

The TryLock class shouldn't be netested.

I'd also add a factory method to it and make it much simpler like this and create an instance only when a lock could be accuired:

private class TryLock : IDisposable
{
    private object _locked;

    private TryLock(object obj)
    {       
        _locked = obj;
    }

    public static TryLock Create(object obj)
    {
        return Monitor.TryEnter(obj) ? new TryLock(obj) : null;
    }

    public void Dispose()
    {
        Monitor.Exit(_locked);
    }
}

Later instead of checking the property you check if the instance isn't null:

using (var tryLock = TryLock.Create(_executeLock))
{
    if (tryLock == null)
    {
        return;
    }

    ...
}