-
Notifications
You must be signed in to change notification settings - Fork 740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Array.Empty<T>() #1210
Use Array.Empty<T>() #1210
Conversation
Use Array.Empty<string>() instead of allocating a new array if either of the strings for comparison are null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to your change, but when you're on it...
@@ -41,7 +41,7 @@ public int Compare(string x, string y) | |||
var xIsInt = x != null && int.TryParse(x, out value1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var xIsInt = int.TryParse(x, out int value1);
var yIsInt = int.TryParse(y, out int value2);
The null-check is done in TryParse
, so here it is redundant. Variable initialized by the out
directly.
var xParts = x?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? new string[0]; | ||
var yParts = y?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? new string[0]; | ||
var xParts = x?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>(); | ||
var yParts = y?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>(); | ||
|
||
// Compare each part until we get two parts that are not equal | ||
for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) | |
var n = Math.Min(xParts.Length, yParts.Length); | |
for (var i = 0; i < n; i++) |
Otherwise n
is evaluated at each iteration.
@@ -26,8 +26,8 @@ public class ConfigurationKeyComparer : IComparer<string> | |||
/// <returns></returns> | |||
public int Compare(string x, string y) | |||
{ | |||
var xParts = x?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? new string[0]; | |||
var yParts = y?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? new string[0]; | |||
var xParts = x?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a common case that either x
or y
is null
?
If so, it may be better to hold Array.Empty<string>
in a field, as the call to Empty
won't be inlined (last time I checked this was true).
If not, nevermind this comment.
Thanks |
Use Array.Empty<string>() instead of allocating a new array if either of the strings for comparison are null. Commit migrated from dotnet/extensions@afb772f
Use Array.Empty<string>() instead of allocating a new array if either of the strings for comparison are null. Commit migrated from dotnet/extensions@afb772f
Use Array.Empty<string>() instead of allocating a new array if either of the strings for comparison are null. Commit migrated from dotnet/extensions@afb772f
Use Array.Empty<string>() instead of allocating a new array if either of the strings for comparison are null. Commit migrated from dotnet/extensions@afb772f
Use
Array.Empty<string>()
instead of allocating a new array if either of the strings for comparison are null.