Dp 2398 tind spread validate header#43
Conversation
davezuckerman
requested review from
Copilot and
steve-sullivan
and removed request for
Copilot
steve-sullivan
left a comment
steve-sullivan
left a comment
There was a problem hiding this comment.
Overall looks good... I only have that one question regarding the regex, but not sure it's even necessary.
|
|
||
| # validates the header rown | ||
| def self.valid_header?(str) | ||
| str.match?(/\d{3}[_|\d]{2}[a-zA-Z0-9]$/) || str.match?(/\d{3}[_|\d]{2}[a-zA-Z0-9]-\d$/) || str.match?(/Filename|FFT/i) |
There was a problem hiding this comment.
Might want to elaborate on the comment (include examples of what the REGEX is matching there)
I'm not 100% sure, I'm rusty w/regular expressions, but is the first 1s match necessary? Could this be slightly simplified by removing the first match and change the 2nd match by wrapping the -d in parens and adding a '?' after it?
example: ...a-zA-Z0-9?$/
There was a problem hiding this comment.
Good point about the comments. Just added some more.
For the reg expression having a ? at the end would work but be too permissive. The option (-1) has to be a dash followed by a number
There was a problem hiding this comment.
so (-\d)? wouldn't work....?
There was a problem hiding this comment.
Actually if I add a "$" after the ? looks like it works.
"/\d{3}[_|\d]{2}a-zA-Z0-9?$/)"
I'll change the expression
2 participants
No description provided.