Removed unnecessary Regex constructors for #353 #354
Conversation
Pulling latest from MehdiK/Humanizer
| @@ -55,8 +60,7 @@ public static string Humanize(this string input) | |||
|
|
|||
| // if input contains a dash or underscore which preceeds or follows a space (or both, i.g. free-standing) | |||
| // remove the dash/underscore and run it through FromPascalCase | |||
| Regex r = new Regex(@"[\s]{1}[-_]|[-_][\s]{1}", RegexOptions.IgnoreCase); | |||
| if (r.IsMatch(input)) | |||
| if (Regex.IsMatch(input, @"[\s]{1}[-_]|[-_][\s]{1}", RegexOptions.IgnoreCase)) | |||
There was a problem hiding this comment.
Why not replace this one with a static version too?
There was a problem hiding this comment.
That's definitely an option, for consistency. I didn't just because it was a shorter regex to inline.
I've made that change in my branch, as well as a few others for even more performance improvements. Should I make a new pull request with those, or can they be added here somehow? (Still pretty new to the whole pull request thing.)
There was a problem hiding this comment.
By simply pushing more commits to your PR branch adds them here.
You already got managed this.
…aster, ToTitleCase.Transform now 2x faster.
| static StringHumanizeExtensions() | ||
| { | ||
| PascalCaseWordPartsRegex = new Regex(@"[A-Z]?[a-z]+|[0-9]+|[A-Z]+(?=[A-Z][a-z]|[0-9]|\b)", | ||
| RegexOptions.IgnorePatternWhitespace | RegexOptions.ExplicitCapture); |
There was a problem hiding this comment.
Should be RegexOptions.Compiled as well
There was a problem hiding this comment.
Agreed, but that option is not supported in portable class libraries, so we're stuck with interpreted expressions. This could be worked around by removing the regex object entirely and "pre-compiling" the expression to a function ourselves, but I'm not sure the added complexity would be worth the performance. Perhaps I'll give it a try, just to see what benefit there is.
|
Thanks for the fix. This is merged now. In the future, please rebase your code before submitting a PR. Also you should add your PR to the release-notes file which I did for this PR. |
|
Thanks for the contribution. This is now available on NuGet as v1.32. |
As per #353,
StringHumanizeExtensions.HumanizeandStringHumanizeExtensions.FromPascalCaseno longer call the Regex constructor on each call.Humanizeuses the staticRegex.IsMatchmethod, andFromPascalCaseinstantiates its needed Regex once during the static constructor. See the new call traces below (taken from running the unit tests.)