Refactor '_validateURI' to use dynamic RegExp #216
Conversation
| } | ||
| const char = isStrictValidation ? '[0-9a-zA-Z_]' : '[^\\s.#]'; | ||
| const dots = isPatternBased ? '\\.{1,2}' : '\\.'; | ||
| const uriPattern = new RegExp(`^(${char}+${dots})*${char}+$`); |
There was a problem hiding this comment.
Hm... I was looking at this a few times, and think that the original variant is a little easier to read/follow. Because here there are a few inline checks first, then Regexp is constructed via template string. So to figure out what flow will be — you have to look at regexp, destruct the template there, then holding that in mind try to compile in the head what values will be for each template parameter...
So at least, it is hard to construct and see the whole regexp. I don't talk about debugging.
There was a problem hiding this comment.
Hmm I see your point, my initial idea was to make the differences between each regex line easier to spot since they had a lot similarities. So by doing this it's easier to see that the char matcher and the dots matcher are the ones that are changing. But I agree the flow has been hurt.
One alternative is to add comments with examples of URIs that match each type of validation, but if you feel better keeping the current version I'm fine with that 😃
Description, Motivation and Context
Refactor '_validateURI' to use dynamic RegExp
What kind of change does this PR introduce?
Checklist: