CODEWITHSUNDEEP
c#stringlinq
nemo_87
nemo_87

Reputation: 4781

Check if string exists in list of strings in C#

Hello I am trying to find best and most efficient way to preform if string exists in list of strings.

My scenario is like this:

  1. Get all string values from database that are satisfying given conditions.
  2. Create random code that has 6 digits, but it is a string value.
  3. Check if generated code exists in list of strings. If it does preform generating of code again, and do it until you find unique string that does not exists in list of strings. When you find one, return it.

This is my code:

private static readonly string chars = "0123456789";

string IGenerateOtpCodeService.GenerateOtpCode()
{
    var otps = personalTestSessionRepository
        .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
        .Select(x => x.Person.Otp)
        .ToList()
        .Distinct();

    Random random = new Random();

    string otp = new string(Enumerable.Repeat(chars, 6)
        .Select(s => s[random.Next(s.Length)]).ToArray());

    //preform check if otp exitst in otps list. if it does, generate otp again, else return otp
    return otp;
}

What is best way to do this? Is it while loop, some LINQ expresion, or something else?

Upvotes: 3

Views: 2366

Answers (6)

MakePeaceGreatAgain
MakePeaceGreatAgain

Reputation: 37000

Instead of looking again and again until a value does not exist, why not simply reverse the logic and take all the numbers that already exist in your database? Order them and increment the greatest one by one to get a number that doesn´t exist:

var ordered = personalTestSessionRepository
    .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
    .Select(x => x.Person.Otp)
    .Order(x => x);

var newNumber = ordered.Last().Otp + 1;

Or when Otp is a string convert to a number first:

var newNumber = (Convert.ToInt32(ordered.Last().Otp) + 1).ToString();

This approach might be faster when nearly all numbers are already in use, so guessing any arbitrary one might take quite long as you may guess the same number multiple times.

EDIT: Alternativly you may simply use Max(x => Convert.ToInt32(x.Otp)) + 1.

Upvotes: 2

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186668

First of all, searching in a List has O(N) time complexity when in case of HashSet is just O(1):

 var ops = new HashSet<int>(personalTestSessionRepository
   .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
   .Select(x => x.Person.Otp));

Next, put Random off the method (or yor can have it badly skewed). Let's work with Random as integer and only then turn it into String

 // Simplest, but not thread safe
 private static Random random = new Random();

 string IGenerateOtpCodeService.GenerateOtpCode() {
   var ops = new HashSet<int>(personalTestSessionRepository
     .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
     .Select(x => x.Person.Otp));

   int value = -1;

   while (true) {
     value = random.Next(10000000);

     if (!ops.Contains(value)) 
       return value.ToString();
   }
 }

If you want just any number, not necessarily random one, you can look for a just first hole: 

string IGenerateOtpCodeService.GenerateOtpCode() {
   var ops = personalTestSessionRepository
     .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
     .Select(x => x.Person.Otp)
     .OrderBy(x => x);

   bool first = true;
   int prior = -1;

   foreach (var item in ops) {
     if (!first && item != prior + 1)
       return item.ToString();

     first = false;
     prior = item; 
   }    

   // no holes, we might want to return the last item + 1
   return (prior + 1).ToString();
}

Upvotes: 2

Niyoko
Niyoko

Reputation: 7662

You should use Distinct before ToList to perform Distinct operation on database server. Then you can check existence of string using Any.

var otps = personalTestSessionRepository
    .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
    .Select(x => x.Person.Otp)
    .Distinct();
    .ToList();

string otp = null;
var found = true; 
do
{
    otp = new string(Enumerable.Repeat(chars, 6)
    .Select(s => s[random.Next(s.Length)]).ToArray());

    found = otps.Any(x=>x == otp);

} while(found)

return otp;

Based on heinzbeinz suggestion, the code will perform faster if you use HashSet. To use HashSet, use code below

var otpsList = personalTestSessionRepository
    .FindAll(x => x.State == PersonalTestSessionStates.NotStarted)
    .Select(x => x.Person.Otp)
    .Distinct();
    .ToList();

var otps = new HashSet<string>(otpsList);

string otp = null;
var found = true; 
do
{
    otp = new string(Enumerable.Repeat(chars, 6)
    .Select(s => s[random.Next(s.Length)]).ToArray());

    found = otps.Contains(otp);

} while(found)

return otp;

Upvotes: 3

Dimitar Tsonev
Dimitar Tsonev

Reputation: 3882

One approach is to sort the list in the database. When you have the sorted list, you can simply use Binary search for checking if the string exist.

Upvotes: 0

selami
selami

Reputation: 2498

Add while loop

    while (true)
    {
        string otp = new string(Enumerable.Repeat(chars, 6)
        .Select(s => s[random.Next(s.Length)]).ToArray());

        if (otps.Contains(otp))
        {
            return otp;
        }
    }

Upvotes: 0

Ilya Chernomordik
Ilya Chernomordik

Reputation: 30175

Well, depending on the number of the strings in the database the best option might be to find it right there in db, instead of loading all of the data to the client and searching for the string afterwards. Yes, this might be easier to write, but if you have lots of strings then you should query the database for it. Of course you should have appropriate index to ensure it's fast.

Upvotes: 0

Related Questions