resolving issue 93. finding valid ip address #96
Conversation
|
@devbazregari : I see you closed #94 - no harm done, but in the future, it would have been best to keep that PR open and make your updates on that PR. In that way, we can see the feedback (the PR reviews) and your improvements all in one place. So I'm going to review this PR, but please keep this PR open and just make your updates on this PR. If you have any questions about this workflow, please let me know and I'm happy to clarify. |
There was a problem hiding this comment.
Thanks for updating this PR! Once you update your regex as described in the comment below, can you replace the one here with your regex? Then push your changes and we can see if the tests pass.
Thanks!
|
|
||
| test_str = 'hi there 153,252,251,252' | ||
|
|
||
| pattern = re.compile(r"\b((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\,|$)){4}\b") |
There was a problem hiding this comment.
Nice! Looks like you found a good way to remove duplicative content from the regex.
Looks like there's one more update needed. This regex will not properly handle strings like:
255,255,255,255 256,256,256,256
(I expect the regex to find 255,255,255,255 in 255,255,255,255 256,256,256,256, but it does not).
So we should modify the regex to this:
| pattern = re.compile(r"\b((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\,|$)){4}\b") | |
| pattern = re.compile(r"\b((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(?:,|\b)){4}") |
Basically, I'm replacing $ with \b to capture any whitespace as the end of the regex. The (?:___) construct means this group is non-capturing (you can read more here).
|
Fixes #93 |
|
Hi @devbazregari : Are you still interested in working on this PR? |
resolving issue 93. finding the valid IP address.
improving issue 93.