-
-
Notifications
You must be signed in to change notification settings - Fork 501
112 Create skeleton for regex #118
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
Conversation
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.
@Meitoseshifu very nice exercise overall 👍
Please take a look and address my comments. Next time I guess it makes sense to create a completed PR as well.
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.
Objectives mean the learning goals of this exercise. What you specified here is prerequisites
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.
@Meitoseshifu every javadoc starts from Create Pattern that accepts String with regex inside and can * find which makes it redundant. You can simplify it, e.g. A pattern that finds a specific word.
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.
@Meitoseshifu it doesn't make sense to have the same
@return Pattern with regex expression
Let's replace it with @return a compiled Pattern instance that looks for the word "Curiosity" or
@return a pattern that looks for the word "Curiosity"
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.
@Meitoseshifu I believe it makes sense to provide some examples inside javadoc. A text and then what should be the result if you use this pattern to find the matches. In this method, I wasn't sure what means "all numbers". E.g. if I have in the text 25th of October then should "25" be found or not? It can be explained in the example.
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.
@Meitoseshifu since it's not necessarily clear what is a zip code, it would be nice to have an example here as well.
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.
Or at least explanation: "a zip code is a 5-digit number without any characters or special symbols,"
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.
@Meitoseshifu the test will pass with this regex [A-Z]{2}. I'm not sure if this is what you've been looking for.
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.
It would be good to have an example.
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.
@Meitoseshifu it would be good to have an example, because it's not clear why links are inside ().
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.
@Meitoseshifu why square brackets are also a part of the result? I guess we should find only the resource e.g. Google, and not [Google].
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.
@Meitoseshifu same here, why (https://google.com) not https://google.com?
No description provided.