Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

add IsCountable validator, to enforce type and count elements. #157

Closed
wants to merge 51 commits into from
Closed

Conversation

abacaphiliac
Copy link
Contributor

hi Team,

i created something like this in a private project (to enforce a countable has at least one element) but thought a more configurable version might be useful to others. what do you think?

@weierophinney weierophinney added this to the 2.10.0 milestone May 17, 2017
Updates `Hostname` validator TLD list.
Add PHP 7.2 support, drop HHVM

Conflicts:
	.travis.yml
- Whitespace following `function` keyword
- Uses newlines and indentation to group conditional statements
Updates the zend-session requirement to v2.8+, to ensure compatibility
with PHP 7.2.
@abacaphiliac
Copy link
Contributor Author

thank you for the feedback. i'll get this fixed up tonight.

}

/**
* @expectedException \Zend\Validator\Exception\RuntimeException
Copy link
Member

@froschdesign froschdesign Jul 25, 2017

Choose a reason for hiding this comment

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

$this->sut->setMin(1);
$this->sut->setMax(10);

self::assertTrue($this->sut->isValid(['Foo']), json_encode($this->sut->getMessages()));
Copy link
Member

@froschdesign froschdesign Jul 25, 2017

Choose a reason for hiding this comment

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

Please use $this-> instead of self::, this is the preferred way. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@Ocramius thinks something else 😜

…max in setOptions which is used by the constructor, use getter methods instead of options field in isValid, report NOT_COUNTABLE as a validation error rather than throwing an exception, and use `$this->` instead of `self::` for assertions in test case.
@abacaphiliac
Copy link
Contributor Author

thanks everyone for the review. i think i took care of everything asked. @weierophinney i chose to implement the mutex logic in setOptions because it is invoked from the constructor. my test case covers behavior of the constructor and setOptions.

add IsCountable validator, to enforce type and count elements.
…x value

Since there is no way to clear a `count`, this is the only way to ensure
that behavior remains as expected when merging options from disparate
sources.
@weierophinney
Copy link
Member

@abacaphiliac I've added some commits that do the following:

  • Make having count and one or both of min or max mutually exclusive; setting a count when either min or max are defined, or setting min or max when count is defined will raise an exception now. This prevents problems issues when you merge configuration; if you expect one set of behavior, you should not have a different behavior!
  • Adds documentation.

I'm expecting the only test failure to be due to the hostname TLD list being out-of-date (again! those used to update more on an order of weeks or months, and now it's practically daily!); if that's the case, I can merge this.

weierophinney added a commit that referenced this pull request Jul 26, 2017
weierophinney added a commit that referenced this pull request Jul 26, 2017
@weierophinney
Copy link
Member

Thanks for this great feature, @abracaphiliac! I have merged it to develop for an upcoming 2.10.0 release (likely today or tomorrow).

@abacaphiliac
Copy link
Contributor Author

my pleasure. thank you for the assist : )

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

Successfully merging this pull request may close these issues.

8 participants