HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Generate key from array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromgeneratearraykey

Problem

What do you think about quality of code in this method?

It should:

  • Generate new key based on keys in array



  • The key should start with "TG"



  • Code should find the max number in array keys with "TG-" and increment this number



  • It should have 7 digits



  • No need to check what if there is a key TG9999999, and what if array is empty or doesn't have keys "TG-"



private static string[] allKeys = new string[]
{
    "TG0000006",
    "TG0000026",
    "TG0000086",
    "TG0000796",
    "TG0023106",
    "LG0004406",
    "MS0000796",
    "TT0023106",
    "LK0004406",
};

static string GenerateKey()
{
    var keys = allKeys.Where(k => k.StartsWith("TG"));
    List nums = new List();
    foreach (var key in keys.Select(str => Regex.Match(str, @"\d+").Value))
    {
        int num;
        if (int.TryParse(key, out num))
            nums.Add(num);
    }
    string newNum = (nums.Max() + 1).ToString();
    return "TG" + new string('0', 7 - newNum.Length) + newNum;
}

Solution

-
If you want only the numbers 0..9 to be matched by the Regex and you don't care about for instance some Eastern Arabic numerals like ´٠١٢٣٤٥٦٧٨٩, then you should use an expression like [0-9]+.

-
because by using either the d+ or [0-9]+ expression you already made sure that the var key is a number. Because this number has at the maximum 7 digits, you can simply call Parse() instead of TryParse() which eleminates the usage of the int num.

-
please add always braces {} although they might be optional. This will make your code less error prone.

-
the return statement

return "TG" + new string('0', 7 - newNum.Length) + newNum;


can be simplified by using the PadLeft() method like so

return "TG" +  newNum.PadLeft(7,'0');


-
personally I would just have a if clase for checking the highest number instead of the List together with the call to Max().

Putting this all together leads to

static string GenerateKey()
{
    int max = int.MinValue;
    var keys = allKeys.Where(k => k.StartsWith("TG"));
    foreach (var key in keys.Select(str => Regex.Match(str, @"[0-9]+").Value))
    {
        int num = int.Parse(key);
        if (num > max)
        {
            max = num;
        }
    }

    return "TG" + (max + 1).ToString().PadLeft(7, '0');
}

Code Snippets

return "TG" + new string('0', 7 - newNum.Length) + newNum;
return "TG" +  newNum.PadLeft(7,'0');
static string GenerateKey()
{
    int max = int.MinValue;
    var keys = allKeys.Where(k => k.StartsWith("TG"));
    foreach (var key in keys.Select(str => Regex.Match(str, @"[0-9]+").Value))
    {
        int num = int.Parse(key);
        if (num > max)
        {
            max = num;
        }
    }

    return "TG" + (max + 1).ToString().PadLeft(7, '0');
}

Context

StackExchange Code Review Q#106974, answer score: 5

Revisions (0)

No revisions yet.