-
Notifications
You must be signed in to change notification settings - Fork 165
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
(#1226) Delete Andand introduce ForEach #1272
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1272 +/- ##
============================================
+ Coverage 89.11% 89.26% +0.15%
- Complexity 1676 1678 +2
============================================
Files 281 283 +2
Lines 4033 4035 +2
Branches 212 212
============================================
+ Hits 3594 3602 +8
+ Misses 404 399 -5
+ Partials 35 34 -1
Continue to review full report at Codecov.
|
Job #1272 is now in scope, role is |
This pull request #1272 is assigned to @fanifieiev/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job |
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.
@ryoku Please take a look at the review comments
* There is no thread-safety guarantee. | ||
* | ||
* @param <X> The type to itetare over | ||
* @since 0.44 |
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.
@ryoku I think the version is not correct here. Take a look into pom.xml.
* @exception Exception If fails | ||
*/ | ||
@SafeVarargs | ||
public final void exec(final X... src) throws Exception { |
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.
@ryoku Public methods in a class should only exist if the underlying interface has it. Please remove it.
Read https://www.yegor256.com/2014/11/20/seven-virtues-of-good-object.html#2-he-works-by-contracts
* There is no thread-safety guarantee. | ||
* | ||
* @param <X> The type to itetare over | ||
* @since 0.44 |
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.
@ryoku I think the version is not correct here. Take a look into pom.xml.
/** | ||
* Test case for {@link ForEachInThreads}. | ||
* | ||
* @since 0.44 |
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.
@ryoku I think the version is not correct here. Take a look into pom.xml.
/** | ||
* Test case for {@link ForEach}. | ||
* | ||
* @since 0.44 |
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.
@ryoku I think the version is not correct here. Take a look into pom.xml.
(Proc<Integer>) list::add | ||
).exec(1, 1); | ||
new Assertion<>( | ||
"List does not contain mapped Iterable elements (2)", list, new IsEqual<>( |
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.
@ryoku Fix the indention here.
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
package org.cactoos.iterator; |
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.
@ryoku The class should be located in org.cactoos.func
package.
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
package org.cactoos.iterator; |
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.
@ryoku The class should be located in org.cactoos.func
package.
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
package org.cactoos.iterator; |
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.
@ryoku The class should be located in org.cactoos.func
package.
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
* SOFTWARE. | ||
*/ | ||
package org.cactoos.iterator; |
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.
@ryoku The class should be located in org.cactoos.func
package.
/** | ||
* The proc. | ||
*/ | ||
private final Func<X, Boolean> wrapped; |
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.
@ryoku Please change wrapped
variable name to func
/** | ||
* The proc. | ||
*/ | ||
private final Func<X, Boolean> wrapped; |
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.
@ryoku Please change wrapped
variable name to func
README.md
Outdated
"how", "are", "you" | ||
).value(); | ||
new ForEach<String>( | ||
(String input) -> System.out.printf( |
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.
@ryoku Please change to input -> System.out.printf(
public void testProcIterable() throws Exception { | ||
final List<Integer> list = new LinkedList<>(); | ||
new ForEach<Integer>( | ||
(Proc<Integer>) list::add |
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.
@ryoku Please change to list:add
only
) | ||
); | ||
new Assertion<>( | ||
"List does not contain mapped Iterable elements (1)", list, |
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.
@ryoku Please move list
to new line
new Assertion<>( | ||
"List does not contain mapped Iterable elements (1)", list, | ||
new IsIterableContainingInAnyOrder<Integer>( | ||
new ListOf<Matcher<? super Integer>>( |
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.
@ryoku Please replace <Matcher<? super Integer>>
with <>
. It is redundant.
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.
@ryoku Please take a look at the review comments once again.
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.
@ryoku Good
@paulodamaso The review process is completed. We are good to merge this PR. Please merge.
@paulodamaso Please merge |
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 25s)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 23s)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 23s)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 23s)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 22s)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 22s)
|
@rultor merge |
@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here |
@paulodamaso Done! FYI, the full log is here (took me 17min) |
@sereshqua/z please review this job completed by @fanifieiev/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
Code review was too long (20 days), architects (@paulodamaso) were penalized, see §55 |
Payment to |
@0crat quality good |
Order was finished, quality is "good": +20 point(s) just awarded to @fanifieiev/z |
Quality review completed: +4 point(s) just awarded to @sereshqua/z |
@rultor release tag is |
@paulodamaso OK, I will release it now. Please check the progress here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 6min)
|
@rultor release tag is |
@paulodamaso OK, I will release it now. Please check the progress here |
@paulodamaso @ryoku Oops, I failed. You can see the full log here (spent 6min)
|
This is for issue (#1226)
Extracted
ForEach
fromAnd
Added
ForEachTest
Extracted
ForEachInThreads
fromAndInThreads
Added
ForEachInThreadsTest
Updated README.md accordingly