>Hi all,
>
>I have this method I am refactoring for various reasons. There is a lot of nesting and plan to remove some of it. I am wondering about this intital test inside of a lock statement and wonder if it would be safe to move it outside of the lock construct.
>
>This is what it looks like now.
>

>public void FindActiveWebService()
>{
>    lock (FindWebServiceSync)
>    {
>        if (!IsFindingActiveWebService)
>        {
>            IsFindingActiveWebService = true;
>            try
>            {
>                // Lots of crappy code and nested code in here with loops and such.
>                // Planning to refactor all this junk.
>            }
>            finally
>            {
>                IsFindingActiveWebService = false;
>            }
>        }
>    }
>}
>

>
>Wondering if it would be just as safe to do this and remove the extra if structure.
>

>public void FindActiveWebService()
>{
>    // Moved outside the lock
>    if (IsFindingActiveWebService)
>        return;
>
>    lock (FindWebServiceSync)
>    {
>        IsFindingActiveWebService = true;
>        try
>        {
>            // Lots of crappy code and nested code in here with loops and such.
>            // Planning to refactor all this junk.
>        }
>        finally
>        {
>            IsFindingActiveWebService = false;
>        }
>    }
>}
>

>
>Thanks for any thoughts.
>Timothy

I just did a find on the usage of "IsFindingActiveWebService" and it isn't used anywhere except this method. Since there is a lock, what the heck value is added by IsFindingActiveWebService - None! Even if there was, why wouldn't the lock be good enough? Doesn't a lock block until the current use is freed? I sometimes think the challenge of fixing bad code can be more than creating some to begin with.