Skip to main content
Code Review

Return to Answer

markdown fix
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396
  • 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 ́٠١٢٣٤٥٦٧٨٩you should use an expression like ́٠١٢٣٤٥٦٧٨٩[0-9]+`, 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<int> 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');
}
  • 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 ́٠١٢٣٤٥٦٧٨٩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<int> 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');
}
  • 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<int> 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');
}
Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177
  • 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 ́٠١٢٣٤٥٦٧٨٩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<int> 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');
}
lang-cs

AltStyle によって変換されたページ (->オリジナル) /