Recursion has its uses but you shouldn't use it in this case.
Here's an article discussing recursive versus iterative solutions:
Rethinking the Practicalities of Recursion[
^]
And here's a forum thread discussing the use of recursion in production code:
Don't ever use recursion in production code?[
^]
In your case, recursion has no benefit. Instead, you risk a stack overflow in case random.Next(..) not yielding an unused FacilityId while there's space on the stack.
Alternative 1: Neither recursion nor iteration
Generate your FacilityIds sequentially instead of randomly: A10000, A10001, .., A99999, B10000, etc.
Then you can just query for the highest FacilityId in use and calculate the next one like this:
string GetNextFacilityId()
{
lock (syncLock)
{
string highestFacilityIdInUse =
objDB.SelectAll()
.OrderByDescending(a => a.FacilityID)
.Select(a => a.FacilityID)
.Take(1)
.SingleOrDefault();
string nextFacilityId;
if (highestFacilityIdInUse == null)
nextFacilityId = "A10000";
else
{
char c = highestFacilityIdInUse[0];
int n = Int32.Parse(highestFacilityIdInUse.Substring(1));
if (c == 'Z' && n == 99999)
throw new InvalidOperationException("ran out of FacilityIds");
else if (n == 99999)
nextFacilityId = (char)(c+1) + "10000";
else
nextFacilityId = c + (n+1).ToString();
}
return nextFacilityId;
}
}
Regarding SelectAll(): I don't know which kind of database access technology you're using here. In this query, SelectAll() might be required, superfluous or completely wrong. Please try with and without it. (Applies also to the code below.)
Alternative 2: Iteration
string GetNextFacilityId()
{
lock (syncLock)
{
Random rnd = new Random();
string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
char c = chars[rnd.Next(chars.Length)];
int n = rnd.Next(10000, 100000);
string nextFacilityId = c + n.ToString();
while (objDB.SelectAll().Any(a => a.FacilityID == nextFacilityId))
{
c = chars[rnd.Next(chars.Length)];
n = rnd.Next(10000, 100000);
nextFacilityId = c + n.ToString();
}
return nextFacilityId;
}
}
Unrelated suggestion: Don't use a method name like "DuplicateCheck". The name doesn't imply what it will do if a duplicate is found or not found. A name like "IsDuplicate" implies that it will return true or false. Likewise (or "worse"), a variable name like "Flag" doesn't say anything (what would Flag == false mean?). It would be more intuitive if it was named "isDuplicate". You will notice that intuitive names for identifiers help a lot when you come back to your code after not looking at it for some weeks :)
Having said that, I would also rename the above identifiers "c" and "n" to something more meaningful like "prefix" and ... well.. "number" or so.. ;)