(#1212) Fixed SetOf, which discarded all elements after duplicated occurrence #1215

fanifieiev · 2019-11-01T14:58:07Z

This pull request is for issue 1212

org.cactoos.set.SetOf was causing an issue when the original collection/array contained duplicate values, which resulted the cut off of the collection just right after first duplicated value.

… one

0crat · 2019-11-01T14:58:46Z

Job #1215 is now in scope, role is REV

codecov-io · 2019-11-01T15:02:54Z

Codecov Report

Merging #1215 into master will increase coverage by 0.14%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #1215      +/-   ##
============================================
+ Coverage     89.22%   89.37%   +0.14%     
- Complexity     1660     1671      +11     
============================================
  Files           279      280       +1     
  Lines          3992     4008      +16     
  Branches        213      213              
============================================
+ Hits           3562     3582      +20     
+ Misses          396      392       -4     
  Partials         34       34

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/cactoos/set/SetOf.java	`100% <100%> (ø)`	`3 <0> (ø)`	⬇️
src/main/java/org/cactoos/scalar/Solid.java	`90% <0%> (-10%)`	`3% <0%> (-1%)`
src/main/java/org/cactoos/io/ReaderOf.java	`96.55% <0%> (-3.45%)`	`28% <0%> (-1%)`
src/main/java/org/cactoos/list/Immutable.java	`100% <0%> (ø)`	`27% <0%> (ø)`	⬇️
...n/java/org/cactoos/list/ImmutableListIterator.java	`91.66% <0%> (ø)`	`9% <0%> (?)`
src/main/java/org/cactoos/list/ListIteratorOf.java	`95.23% <0%> (+0.79%)`	`12% <0%> (ø)`	⬇️
...ain/java/org/cactoos/list/ListIteratorNoNulls.java	`85.71% <0%> (+38.09%)`	`9% <0%> (+4%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e0da30...7f5640a. Read the comment docs.

0crat · 2019-11-01T15:13:16Z

This pull request #1215 is assigned to @fabriciofx /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

victornoel · 2019-11-02T16:08:08Z

src/main/java/org/cactoos/set/SetOf.java

@@ -60,7 +61,7 @@ public SetOf(final T... array) {
    public SetOf(final Iterable<T> src) {
        super(() -> {
            final Set<T> tmp = new HashSet<>();
-            new And(tmp::add, src)
+            new And((Proc<T>) tmp::add, src)


@fanifieiev I think it would be way better to use src.forEach(tmp::add), that's what Iterable is made for.

@victornoel Sure, that one looks better. But just wondering what was the And(final Proc<X> proc, final Iterable<X> src) created for ?

@fanifieiev I think it is here only for completeness. And is more about the logical assertion (which does not rely on side effect as the Proc version does).

I created #1226 in reaction to this discussion :)

fanifieiev · 2019-11-06T16:45:02Z

@0crat status

0crat · 2019-11-06T16:52:38Z

@0crat status (here)

@fanifieiev This is what I know about this job in C63314D6Z, as in §32:

The job #1215 is in scope for 5days
The role is REV
The job is assigned to @fabriciofx /z for 5days
There is no monetary reward attached, it's a free job
The job doesn't have any impediments
The budget is 15 minutes/points
These users are banned and won't be assigned:
- @fanifieiev /z: This user reported the ticket
Job footprint (restricted area)

fanifieiev · 2019-11-07T03:02:38Z

@0crat wait for REV

0crat · 2019-11-07T04:15:29Z

@0crat wait for REV (here)

@fanifieiev @fanifieiev /z you can't register impediment for this job

fanifieiev · 2019-11-11T16:26:40Z

@0crat status

0crat · 2019-11-11T16:27:56Z

@0crat status (here)

@fanifieiev This is what I know about this job in C63314D6Z, as in §32:

The job #1215 is in scope for 10days
The role is REV
The job is assigned to @fabriciofx /z for 10days
There is no monetary reward attached, it's a free job
The job doesn't have any impediments
The budget is 15 minutes/points
These users are banned and won't be assigned:
- @fanifieiev /z: This user reported the ticket
Job footprint (restricted area)

fanifieiev · 2019-11-11T16:34:03Z

@paulodamaso This PR review is taking really long.

fabriciofx

@fanifieiev please, take a look.

fabriciofx · 2019-11-12T02:59:38Z

src/test/java/org/cactoos/set/SetOfTest.java

-            "Can't behave as a set",
+    public void behaveAsSetWithOriginalDuplicationsInTheTail() {
+        new Assertion<>(
+            "Must keep unique numbers",


@fanifieiev please, change "Must keep unique numbers" to "Must keep unique integer numbers".

fabriciofx · 2019-11-12T02:59:47Z

src/test/java/org/cactoos/set/SetOfTest.java

+    @Test
+    public void behaveAsSetWithOriginalDuplicationsInTheHead() {
+        new Assertion<>(
+            "Must keep unique numbers",


@fanifieiev please, change "Must keep unique numbers" to "Must keep unique integer numbers".

fabriciofx · 2019-11-12T02:59:55Z

src/test/java/org/cactoos/set/SetOfTest.java

+    @Test
+    public void behaveAsSetWithOriginalDuplicationsInTheMiddle() {
+        new Assertion<>(
+            "Must keep unique numbers",


@fanifieiev please, change "Must keep unique numbers" to "Must keep unique integer numbers".

fabriciofx · 2019-11-12T03:01:34Z

src/test/java/org/cactoos/set/SetOfTest.java

+    }
+
+    @Test
+    public void behaveAsSetWithOriginalMergedCollectionsWithDuplicates() {


@fanifieiev please, add more tests for:

chars

double numbers

other type object different of primitives/boxed and string.

fanifieiev · 2019-11-12T04:11:25Z

@fabriciofx The review issues have been resolved. Please review.

fabriciofx · 2019-11-12T12:02:53Z

@fanifieiev seems fine to me. @paulodamaso can you merge it, please?

fanifieiev · 2019-11-13T20:35:03Z

@paulodamaso Can you please merge it? Please see

fabriciofx · 2019-11-16T16:43:00Z

@paulodamaso ping

paulodamaso · 2019-11-18T18:17:39Z

@rultor merge

rultor · 2019-11-18T18:18:35Z

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2019-11-18T18:32:22Z

@rultor merge

@paulodamaso Done! FYI, the full log is here (took me 13min)

0crat · 2019-11-18T18:56:12Z

@sereshqua /z please review this job completed by @fabriciofx /z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

0crat · 2019-11-18T18:56:28Z

Code review was too long (17 days), architects (@paulodamaso) were penalized, see §55

0crat · 2019-11-18T18:57:05Z

The job #1215 is now out of scope

0crat · 2019-11-18T18:58:20Z

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @paulodamaso /z

sereshqua · 2019-11-18T20:17:16Z

@fabriciofx please make sure next CR your comments would be mostly about design problems, not just the cosmetic issues

fabriciofx · 2019-11-18T22:25:04Z

@sereshqua ok. Thanks!

sereshqua · 2019-11-19T05:34:02Z

@0crat quality acceptable

0crat · 2019-11-19T05:51:23Z

Quality review completed: +4 point(s) just awarded to @sereshqua /z

0crat · 2019-11-19T05:53:33Z

Order was finished, quality is "acceptable": +15 point(s) just awarded to @fabriciofx /z

(yegor256#1212) Fixed SetOf, which removed all elems after duplicated…

b0c6c77

… one

0crat added the scope label Nov 1, 2019

fanifieiev mentioned this pull request Nov 1, 2019

(#1212) Fix Setof implementation and test #1213

Closed

(yegor256#1212) Fixed tests to do proper checks

0d75b50

victornoel reviewed Nov 2, 2019

View reviewed changes

(yegor256#1212) Refactoring due to code review remarks

77b0162

victornoel mentioned this pull request Nov 4, 2019

Delete And(Proc, ...) and introduce ForEach(Proc, ...) #1226

Closed

fanifieiev mentioned this pull request Nov 11, 2019

SetOf discards non-duplicate elements #1212

Closed

fabriciofx suggested changes Nov 12, 2019

View reviewed changes

(yegor256#1212) After-review fixes with additional unit tests

7f5640a

rultor merged commit 7f5640a into yegor256:master Nov 18, 2019

0crat removed the scope label Nov 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(#1212) Fixed SetOf, which discarded all elements after duplicated occurrence #1215

(#1212) Fixed SetOf, which discarded all elements after duplicated occurrence #1215

fanifieiev commented Nov 1, 2019

0crat commented Nov 1, 2019

codecov-io commented Nov 1, 2019 •

edited

Loading

0crat commented Nov 1, 2019

victornoel Nov 2, 2019

fanifieiev Nov 4, 2019

victornoel Nov 4, 2019

fanifieiev commented Nov 6, 2019

0crat commented Nov 6, 2019

fanifieiev commented Nov 7, 2019

0crat commented Nov 7, 2019

fanifieiev commented Nov 11, 2019

0crat commented Nov 11, 2019

fanifieiev commented Nov 11, 2019

fabriciofx left a comment

fabriciofx Nov 12, 2019

fabriciofx Nov 12, 2019

fabriciofx Nov 12, 2019

fabriciofx Nov 12, 2019

fanifieiev commented Nov 12, 2019

fabriciofx commented Nov 12, 2019

fanifieiev commented Nov 13, 2019

fabriciofx commented Nov 16, 2019

paulodamaso commented Nov 18, 2019

rultor commented Nov 18, 2019

rultor commented Nov 18, 2019

0crat commented Nov 18, 2019

0crat commented Nov 18, 2019

0crat commented Nov 18, 2019

0crat commented Nov 18, 2019

sereshqua commented Nov 18, 2019

fabriciofx commented Nov 18, 2019

sereshqua commented Nov 19, 2019

0crat commented Nov 19, 2019

0crat commented Nov 19, 2019

(#1212) Fixed SetOf, which discarded all elements after duplicated occurrence #1215

(#1212) Fixed SetOf, which discarded all elements after duplicated occurrence #1215

Conversation

fanifieiev commented Nov 1, 2019

0crat commented Nov 1, 2019

codecov-io commented Nov 1, 2019 • edited Loading

Codecov Report

0crat commented Nov 1, 2019

victornoel Nov 2, 2019

Choose a reason for hiding this comment

fanifieiev Nov 4, 2019

Choose a reason for hiding this comment

victornoel Nov 4, 2019

Choose a reason for hiding this comment

fanifieiev commented Nov 6, 2019

0crat commented Nov 6, 2019

fanifieiev commented Nov 7, 2019

0crat commented Nov 7, 2019

fanifieiev commented Nov 11, 2019

0crat commented Nov 11, 2019

fanifieiev commented Nov 11, 2019

fabriciofx left a comment

Choose a reason for hiding this comment

fabriciofx Nov 12, 2019

Choose a reason for hiding this comment

fabriciofx Nov 12, 2019

Choose a reason for hiding this comment

fabriciofx Nov 12, 2019

Choose a reason for hiding this comment

fabriciofx Nov 12, 2019

Choose a reason for hiding this comment

fanifieiev commented Nov 12, 2019

fabriciofx commented Nov 12, 2019

fanifieiev commented Nov 13, 2019

fabriciofx commented Nov 16, 2019

paulodamaso commented Nov 18, 2019

rultor commented Nov 18, 2019

rultor commented Nov 18, 2019

0crat commented Nov 18, 2019

0crat commented Nov 18, 2019

0crat commented Nov 18, 2019

0crat commented Nov 18, 2019

sereshqua commented Nov 18, 2019

fabriciofx commented Nov 18, 2019

sereshqua commented Nov 19, 2019

0crat commented Nov 19, 2019

0crat commented Nov 19, 2019

codecov-io commented Nov 1, 2019 •

edited

Loading