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

#22 avoid repetitive call $this->_calculatePageCount() in count() method if count equals zero #26

Merged
merged 2 commits into from
Nov 1, 2017

Conversation

fedys
Copy link
Contributor

@fedys fedys commented Jul 19, 2016

#22

@froschdesign
Copy link
Member

Duplicates #25

@fedys
Copy link
Contributor Author

fedys commented Jul 19, 2016

@froschdesign you are right my PR is duplicating #25 but my PR contains tests which tests the repetitive call of Zend\Paginator\Paginator::_calculatePageCount() unlike the PR #25. Please consider merging of my ZendTest\Paginator\PaginatorTest::testRepetitiveCallCountResultingZeroDoesNotCauseRepetitiveCallOfCalculatePageCountMethod()

@froschdesign
Copy link
Member

I've seen it and I prefer your test in this special case.

@fedys
Copy link
Contributor Author

fedys commented Jul 19, 2016

thanks :)

@froschdesign
Copy link
Member

@fedys
Can you simplify the test method name? Maybe replace "RepetitiveCallCountResultingZero" with "CountResultOfZero" or something shorter.

@froschdesign froschdesign added this to the 2.7.1 milestone Jun 3, 2017
@weierophinney
Copy link
Member

Can you simplify the test method name?

Just a note: as a general rule, the more descriptive the name, the better. Test names should indicate the behavior being tested.

@weierophinney weierophinney merged commit 413abf5 into zendframework:master Nov 1, 2017
weierophinney added a commit that referenced this pull request Nov 1, 2017
\#22 avoid repetitive call $this->_calculatePageCount() in count() method if count equals zero

Conflicts:
	src/Paginator.php
weierophinney added a commit that referenced this pull request Nov 1, 2017
weierophinney added a commit that referenced this pull request Nov 1, 2017
weierophinney added a commit that referenced this pull request Nov 1, 2017
@weierophinney
Copy link
Member

Thanks, @fedys!

@froschdesign froschdesign modified the milestones: 2.7.1, 2.8.0 Jan 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants