(#1560) Delegate `TextOf(Iterator)` to `TextOf(Iterable)` #1599

DmitryBarskov · 2021-05-04T15:20:26Z

Constructor TextOf(Iterator) now delegates to TextOf(Iterable)
Constructor TextOf parameter is typed as Iterable<Character> instead of Iterable<?>
Add extra test for TextOf(Iterable)
Replace usages of TextOf(Iterable<?>) with Joined and Concatenated

victornoel

@DmitryBarskov just a little comment until a REV gets assigned to the PR :)

src/main/java/org/cactoos/text/TextOf.java

andreoss · 2021-05-04T16:38:57Z

src/main/java/org/cactoos/map/MapEnvelope.java

@@ -124,7 +126,14 @@ public final void clear() {
    public final String toString() {
        return new StringBuilder()
            .append('{')
-            .append(new TextOf(this.entrySet()).toString())
+            .append(


@DmitryBarskov Why use Concatenated and StringBuilder together?

Do I do it right now?

@DmitryBarskov yes you should fix the issue the reviewer found, I think here he is hinting at the fact there is useless extra code and that it could be simplified… not 100% sure :)

@victornoel Yes, I got it, thank you. Just wanted to draw attention to the changes.

src/test/java/org/cactoos/text/TextOfTest.java

Co-authored-by: Victor Noël <[email protected]>

Co-authored-by: andreoss <[email protected]>

src/main/java/org/cactoos/text/TextOf.java

src/main/java/org/cactoos/map/MapEnvelope.java

victornoel

@DmitryBarskov a few more comments

victornoel · 2021-05-07T11:04:48Z

src/test/java/org/cactoos/map/MapEnvelopeTest.java

@@ -301,6 +301,18 @@ public void removeIsDelegated() {
        ).affirm();
    }

+    @Test
+    public void testToString() {


@DmitryBarskov let's move this test in MapOfTest: even though it's implemented in MapEnvelope (which is a mistake I think, see #1606), this test is ultimately testing MapOf from a user point of view.

I see there are tests for this method already, I'll just remove it.

victornoel · 2021-05-07T13:37:13Z

src/main/java/org/cactoos/text/TextOf.java

-    public TextOf(final Iterable<?> iterable) {
+     * @todo #1560:30min/DEV We want {@link Concatenated} to have
+     *  an extra constructor that accepts {@code Iterable<CharSequence>}
+     *  to avoid creating of {@link Joined} with empty delimiter.


@DmitryBarskov actually it won't be possible to introduce such constructor because of type erasure, so I propose to simply write the following in the constructor below instead: new Concatenated(new Mapped(TextOf::new, iterable))

victornoel · 2021-05-07T13:37:50Z

src/main/java/org/cactoos/text/TextOf.java

-                }
-            )
-        );
+        this(new IterableOf<>(iterator));


@DmitryBarskov please move this constructor above (good practice is to always have the called constructor/methods below the calling ones)

Co-authored-by: Victor Noël <[email protected]>

victornoel · 2021-05-08T16:53:42Z

@DmitryBarskov @andreoss thx

victornoel · 2021-05-08T16:53:47Z

@rultor merge

rultor · 2021-05-08T16:54:30Z

@rultor merge

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

rultor · 2021-05-08T17:07:22Z

@rultor merge

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

0crat · 2021-05-08T17:28:44Z

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

sereshqua · 2021-05-08T17:47:57Z

@DmitryBarskov please make sure you start all of the comments with the name of the user they are referred to, see

sereshqua · 2021-05-08T17:48:05Z

@0crat quality acceptable

DmitryBarskov added 5 commits May 4, 2021 18:06

(yegor256#1560) Delegate TextOf(Iterable) to TextOf(Iterator)

c266a2d

(yegor256#1560) Test TextOf(Iterable)

43609f5

(yegor256#1560) Replace usage in MapEnvelope::toString

cd07e86

(yegor256#1560) Replace usage in IterableOf::toString

1c58381

(yegor256#1560) Remove the puzzle

40209ef

0crat added 0crat/scope 0crat/role/REV labels May 4, 2021

victornoel requested changes May 4, 2021

View reviewed changes

src/main/java/org/cactoos/text/TextOf.java Outdated Show resolved Hide resolved

andreoss suggested changes May 4, 2021

View reviewed changes

DmitryBarskov and others added 4 commits May 5, 2021 10:55

(yegor256#1560) Delegate TextOf(Iterator) to TextOf(Iterable)

6588220

Co-authored-by: Victor Noël <[email protected]>

(yegor256#1560) Use IsText matcher instead of asString and IsEqual

29f7369

Co-authored-by: andreoss <[email protected]>

(yegor256#1560) Use 'Must' in assertion messages

9a63102

Co-authored-by: andreoss <[email protected]>

(yegor256#1560) Use Concatendated instead of StringBuilder

a8ee3f6

Co-authored-by: andreoss <[email protected]>

DmitryBarskov force-pushed the issue-1560 branch from 4bf9b9a to a8ee3f6 Compare May 5, 2021 08:11

DmitryBarskov commented May 5, 2021

View reviewed changes

src/main/java/org/cactoos/text/TextOf.java Outdated Show resolved Hide resolved

DmitryBarskov commented May 5, 2021

View reviewed changes

src/main/java/org/cactoos/map/MapEnvelope.java Show resolved Hide resolved

(yegor256#1560) Add test for MapEnvelope::toString

0547385

DmitryBarskov changed the title ~~(#1560) Delegate TextOf(Iterable) to TextOf(Iterator)~~ (#1560) Delegate TextOf(Iterator) to TextOf(IteratorIterable) May 5, 2021

DmitryBarskov changed the title ~~(#1560) Delegate TextOf(Iterator) to TextOf(IteratorIterable)~~ (#1560) Delegate TextOf(Iterator) to TextOf(Iterable) May 5, 2021

(yegor256#1560) Add puzzle for Concatenated(Iterable<CharSequence>)

9bfd182

DmitryBarskov requested a review from victornoel May 7, 2021 10:26

victornoel requested changes May 7, 2021

View reviewed changes

DmitryBarskov and others added 3 commits May 7, 2021 23:04

(yegor256#1560) Apply suggestions from code review

9ebfea6

Co-authored-by: Victor Noël <[email protected]>

Merge branch 'master' of github.com:yegor256/cactoos into issue-1560

05ec84d

Address code style offences

91d8840

rultor merged commit 91d8840 into yegor256:master May 8, 2021

0crat added the qa label May 8, 2021

0crat removed the 0crat/scope label May 8, 2021

0crat added quality/acceptable and removed qa labels May 8, 2021

DmitryBarskov deleted the issue-1560 branch May 10, 2021 19:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(#1560) Delegate `TextOf(Iterator)` to `TextOf(Iterable)` #1599

(#1560) Delegate `TextOf(Iterator)` to `TextOf(Iterable)` #1599

DmitryBarskov commented May 4, 2021 •

edited

Loading

victornoel left a comment

andreoss May 4, 2021

DmitryBarskov May 5, 2021

victornoel May 5, 2021

DmitryBarskov May 5, 2021

victornoel left a comment

victornoel May 7, 2021

DmitryBarskov May 7, 2021 •

edited

Loading

victornoel May 7, 2021

victornoel May 7, 2021

victornoel commented May 8, 2021

victornoel commented May 8, 2021

rultor commented May 8, 2021

rultor commented May 8, 2021

0crat commented May 8, 2021

sereshqua commented May 8, 2021

sereshqua commented May 8, 2021

(#1560) Delegate TextOf(Iterator) to TextOf(Iterable) #1599

(#1560) Delegate TextOf(Iterator) to TextOf(Iterable) #1599

Conversation

DmitryBarskov commented May 4, 2021 • edited Loading

victornoel left a comment

Choose a reason for hiding this comment

andreoss May 4, 2021

Choose a reason for hiding this comment

DmitryBarskov May 5, 2021

Choose a reason for hiding this comment

victornoel May 5, 2021

Choose a reason for hiding this comment

DmitryBarskov May 5, 2021

Choose a reason for hiding this comment

victornoel left a comment

Choose a reason for hiding this comment

victornoel May 7, 2021

Choose a reason for hiding this comment

DmitryBarskov May 7, 2021 • edited Loading

Choose a reason for hiding this comment

victornoel May 7, 2021

Choose a reason for hiding this comment

victornoel May 7, 2021

Choose a reason for hiding this comment

victornoel commented May 8, 2021

victornoel commented May 8, 2021

rultor commented May 8, 2021

rultor commented May 8, 2021

0crat commented May 8, 2021

sereshqua commented May 8, 2021

sereshqua commented May 8, 2021

(#1560) Delegate `TextOf(Iterator)` to `TextOf(Iterable)` #1599

(#1560) Delegate `TextOf(Iterator)` to `TextOf(Iterable)` #1599

DmitryBarskov commented May 4, 2021 •

edited

Loading

DmitryBarskov May 7, 2021 •

edited

Loading