1

My ASP.NET application requires me to generate a huge number of random strings such that each contain at least 1 alphabetic and numeric character and should be alphanumeric on the whole. For this my logic is to generate the code again if the random string is numeric:

        public static string GenerateCode(int length)
        {
            if (length < 2 || length > 32)
            {
                throw new RSGException("Length cannot be less than 2 or greater than 32.");
            }
            string newcode = Guid.NewGuid().ToString("n").Substring(0, length).ToUpper();
            return newcode;
        }

        public static string GenerateNonNumericCode(int length)
        {
            string newcode = string.Empty;
            try
            {
                newcode = GenerateCode(length);
            }
            catch (Exception)
            {
                throw;
            }
            while (IsNumeric(newcode))
            {
                return GenerateNonNumericCode(length);
            }
            return newcode;
        }

        public static bool IsNumeric(string str)
        {
            bool isNumeric = false;
            try
            {
                long number = Convert.ToInt64(str);
                isNumeric = true;
            }
            catch (Exception)
            {
                isNumeric = false;
            }
            return isNumeric;
        }

While debugging, it is working properly but when I ask it to create 10,000 random strings, its not able to handle it properly. When I export that data to Excel, I find at least 20 strings on an average that are numeric. Is it a problem with my code or C#? - Mine.
If anyone's looking for code,

public static string GenerateCode(int length)
    {
        if (length < 2)
        {
            throw new A1Exception("Length cannot be less than 2.");
        }
        var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
        var random = new Random();
        var result = new string(

        Enumerable.Repeat(chars, length)
                  .Select(s => s[random.Next(s.Length)])
                  .ToArray());

    return result;
}

public static string GenerateAlphaNumericCode(int length)
{
    string newcode = string.Empty;
    try
    {
        newcode = GenerateCode(length);
        while (!IsAlphaNumeric(newcode))
        {
            newcode = GenerateCode(length);
        }
    }
    catch (Exception)
    {
        throw;
    }

    return newcode;
}

public static bool IsAlphaNumeric(string str)
{
    bool isAlphaNumeric = false;
    Regex reg = new Regex("[0-9A-Z]+");
    isAlphaNumeric = reg.IsMatch(str);
    return isAlphaNumeric;
}

Thanks to all for your ideas.

Null Head
  • 2,877
  • 13
  • 61
  • 83
  • 6
    Just a guess, but it's probably you, not C#. – Patrick87 Jul 26 '11 at 00:13
  • I don't see anything wrong with the code. Well, I do, actually, it's VERY slow for generating a lot of random strings, but I'll get to that later. In the meantime, do you actually have your code to generate the 10,000 strings? I'd like to see it. It seemed to work fine for me. – aquinas Jul 26 '11 at 00:48

5 Answers5

6

use name space then using System.Linq; use normal string check whether the string consist at lest one character or number.

using System.Linq; 

string StrCheck = "abcd123";
check the string has characters --->  StrCheck.Any(char.IsLetter) 

check the string has numbers   --->   StrCheck.Any(char.IsDigit)

if (StrCheck.Any(char.IsLetter) && StrCheck.Any(char.IsDigit))
{
//statement goes here.....
}

sorry for the late reply ...
user3497269
  • 61
  • 1
  • 2
6

If you want to stick with the Guid as the generator, you could always validate using a Regex This will only return true if at least one alpha is present

Regex reg = new Regex("[a-zA-Z]+");

Then just use the IsMatch method to see if your string is valid

That way you don't need the (IMHO rather ugly) try..catch around the Convert.

Update : I see your subsequent comment about actually making your code slower. Are you instantiating the Regex object only once, or every time that the test is being done? If the latter then this will be rather inefficient, and you should consider using a "lazy-loaded" property on your class, e.g.

    private Regex reg;

    private Regex AlphaRegex
    {
        get
        {
            if (reg == null) reg = new Regex("[a-zA-Z]+");
            return reg;
        }
    }

Then just use AlphaRegex.IsMatch() in your method. I would expect this to make a difference.

DeanOC
  • 7,142
  • 6
  • 42
  • 56
  • @Reddy S R: Re your slow-down comment. I see your code does create the Regex object every time. Try the lazy-loaded property to speed things up. – DeanOC Jul 26 '11 at 04:07
2

I didn't quite understand what you want in the string except letters (abc etc) - lets say numbers.
You can generate a random character as following:

Random r = new Random();
r.Next('a', 'z'); //For lowercase
r.Next('A', 'Z'); //For capitals
//or you can convert lowercase to capital:
char c = 'k' + ('A' - 'a');


If you want to create a string:

var s = new StringBuilder();
for(int i = 0; i < length; ++i)
    s.Append((char)r.Next('a', 'z' + 1)); //Changed to char
return s.ToString(); 


Note: I don't know ASP.NET so much, so I just act like it's C#.

Mark Segal
  • 5,427
  • 4
  • 31
  • 69
  • Note that you must cast the result of `r.Next` to `char`, or the values appended to the string will still be numbers rather than letters. – Dan J Jul 26 '11 at 00:25
  • Also, the upper bound of `Random.Next` is exclusive, so if you want z to potentially appear in the string, you'll need to add 1 to that bound. I suppose you could randomly pick whether to generate a random lowercase letter, uppercase letter, or numeral. – Dan J Jul 26 '11 at 00:32
  • @Quantic: I need a alphanumeric string, not just alphabetic. – Null Head Jul 26 '11 at 00:51
  • @Reddy well you don't pay me enough for me to make you all the work – Mark Segal Jul 26 '11 at 01:24
1

To answer your question strictly, using your existing code: there is a problem with your recursion logic, which can be avoided by not using recursion (there is absolutely no reason to use recursion in GenerateNonNumericCode). Do the following instead:

    public static string GenerateNonNumericCode(int length)
    {
        string newcode = GenerateCode(length);
        while (IsNumeric(newcode))
        {
            newcode = GenerateCode(length);
        }
        return newcode;
    }

Other General Notes

Your code is very inefficient--throwing exceptions is expensive, so using try/catch in a loop is therefore slow and pointless. As others have suggested, regex makes more sense (System.Text.RegularExpressions namespace).

Is it a problem with my code or C#?

When in doubt, the problem is almost never C#.

Justin M. Keyes
  • 6,679
  • 3
  • 33
  • 60
  • As a side note, I have changed the code to use Regex instead of try/catch and have noted that the app took 10 seconds longer. – Null Head Jul 26 '11 at 01:31
0

So, I would change the code to this:

static Random r = new Random();
public static string GenerateNonNumericCodeFaster(int length) {
    var firstLength = r.Next(0, length - 1);
    var secondLength = length - 1 - firstLength;
    return GenerateCode(firstLength)
         + (char) r.Next((int)'A', (int)'G')
         + GenerateCode(secondLength);
}

You can keep your GenerateCode function as is. Everything else you toss out. The idea here of course is, rather than testing if the string contains an alphabetic character, you just explicitly PUT one in. In my tests, using this code could generate 10,000 8 character strings in 0.0172963 seconds compared to your code which takes around 52 seconds. So, yeah, this is about 3000 times faster :)

Michael Petrotta
  • 59,888
  • 27
  • 145
  • 179
aquinas
  • 23,318
  • 5
  • 58
  • 81