snippetcsharpMinor
Generate key from array
Viewed 0 times
fromgeneratearraykey
Problem
What do you think about quality of code in this method?
It should:
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
-
because by using either the
-
please add always braces
-
the return statement
can be simplified by using the
-
personally I would just have a
Putting this all together leads to
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.