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<int> nums = new List<int>();
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;
}
1 Answer 1
If you want only the numbers
0..9
to be matched by theRegex
and you don't care about for instance some Eastern Arabic numerals liké٠١٢٣٤٥٦٧٨٩
, then you should use an expression like[0-9]+
.because by using either the
d+
or[0-9]+
expression you already made sure that thevar key
is a number. Because this number has at the maximum 7 digits, you can simply callParse()
instead ofTryParse()
which eleminates the usage of theint 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 soreturn "TG" + newNum.PadLeft(7,'0');
personally I would just have a
if
clase for checking the highest number instead of theList<int>
together with the call toMax()
.
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');
}