Skip to content
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

#80 validate number of closing parenthesis #81

Merged
merged 2 commits into from
Oct 11, 2015
Merged

#80 validate number of closing parenthesis #81

merged 2 commits into from
Oct 11, 2015

Conversation

whyte624
Copy link
Contributor

@whyte624 whyte624 commented Oct 2, 2015

Fix issue #80.
Checking number of closing parenthesis and compare it with number of parenthesis opened before.

*/
protected function parseComments()
protected function parseComments(&$openedParenthesis = 0)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing by reference (which causes hidden side effects), the method can return the number of parenthesis found. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of it, but it looked not obvious for method parseComments to return number of parenthesis.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And making it protected within Parser class? We can initialize it to 0 outside the loop of the Local or Domain parsers.
Similar side-efectish but more OOP.
WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to change the $openParenthesis variable to a protected field?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.
El 5/10/2015 8:01 p. m., "Andrei Sozonov" [email protected]
escribió:

In src/Egulias/EmailValidator/Parser/Parser.php
#81 (comment):

  */
  • protected function parseComments()
  • protected function parseComments(&$openedParenthesis = 0)

Do you mean to change the $openParenthesis variable to a protected field?


Reply to this email directly or view it on GitHub
https://github.com/egulias/EmailValidator/pull/81/files#r41175960.

@whyte624
Copy link
Contributor Author

whyte624 commented Oct 7, 2015

If it's alright now, let's proceed with a release. I would like to have those changes in my project. Thanks!

egulias added a commit that referenced this pull request Oct 11, 2015
#80 validate number of closing parenthesis
@egulias egulias merged commit 5e91e29 into egulias:master Oct 11, 2015
@egulias
Copy link
Owner

egulias commented Oct 11, 2015

Sorry for the delay.

@egulias
Copy link
Owner

egulias commented Oct 11, 2015

1.2.10 is out with your changes.

@whyte624
Copy link
Contributor Author

Great! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants