Skip to content
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.

Sign up for GitHub

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

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

Merged
merged 14 commits into from
May 8, 2021
Merged

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

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c266a2d
(#1560) Delegate TextOf(Iterable) to TextOf(Iterator)
DmitryBarskov May 4, 2021
43609f5
(#1560) Test TextOf(Iterable)
DmitryBarskov May 4, 2021
cd07e86
(#1560) Replace usage in MapEnvelope::toString
DmitryBarskov May 4, 2021
1c58381
(#1560) Replace usage in IterableOf::toString
DmitryBarskov May 4, 2021
40209ef
(#1560) Remove the puzzle
DmitryBarskov May 4, 2021
6588220
(#1560) Delegate TextOf(Iterator) to TextOf(Iterable)
DmitryBarskov May 5, 2021
29f7369
(#1560) Use IsText matcher instead of asString and IsEqual
DmitryBarskov May 5, 2021
9a63102
(#1560) Use 'Must' in assertion messages
DmitryBarskov May 5, 2021
a8ee3f6
(#1560) Use Concatendated instead of StringBuilder
DmitryBarskov May 5, 2021
0547385
(#1560) Add test for MapEnvelope::toString
DmitryBarskov May 5, 2021
9bfd182
(#1560) Add puzzle for Concatenated(Iterable<CharSequence>)
DmitryBarskov May 5, 2021
9ebfea6
(#1560) Apply suggestions from code review
DmitryBarskov May 7, 2021
05ec84d
Merge branch 'master' of github.com:yegor256/cactoos into issue-1560
DmitryBarskov May 7, 2021
91d8840
Address code style offences
DmitryBarskov May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/main/java/org/cactoos/iterable/IterableOf.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.cactoos.scalar.ScalarWithFallback;
import org.cactoos.scalar.SumOfInt;
import org.cactoos.scalar.Unchecked;
import org.cactoos.text.TextOf;
import org.cactoos.text.Joined;
import org.cactoos.text.UncheckedText;

/**
Expand Down Expand Up @@ -134,6 +134,14 @@ public int hashCode() {

@Override
public String toString() {
return new UncheckedText(new TextOf(this)).asString();
return new UncheckedText(
new Joined(
", ",
new Mapped<>(
Object::toString,
this
)
)
).asString();
}
}
16 changes: 11 additions & 5 deletions src/main/java/org/cactoos/map/MapEnvelope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import java.util.Collection;
import java.util.Map;
import java.util.Set;
import org.cactoos.iterable.Mapped;
import org.cactoos.text.Concatenated;
import org.cactoos.text.Joined;
import org.cactoos.text.TextOf;

/**
Expand Down Expand Up @@ -122,11 +125,14 @@ public final Set<Map.Entry<X, Y>> entrySet() {

@Override
public final String toString() {
return new StringBuilder()
.append('{')
.append(new TextOf(this.entrySet()).toString())
.append('}')
.toString();
return new Concatenated(
new TextOf("{"),
new Joined(
", ",
new Mapped<>(Object::toString, this.entrySet())
),
new TextOf("}")
).toString();
}

@Override
Expand Down
26 changes: 8 additions & 18 deletions src/main/java/org/cactoos/text/TextOf.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.cactoos.Text;
import org.cactoos.bytes.BytesOf;
import org.cactoos.io.InputOf;
import org.cactoos.iterable.IterableOf;
import org.cactoos.iterable.Mapped;

/**
Expand Down Expand Up @@ -317,17 +318,14 @@ public TextOf(final String input) {
* Ctor.
*
* @param iterable The iterable to convert to string
* @todo #1461:30min We want constructors with {@code Iterable<?>} and
* {@code Iterator<Character>} to have same behaviour (simply concatenate list of strings).
* To do that, we should change {@code Iterable<?>} to {@code Iterable<Character>}
* and delegate the {@link Iterator} one to the {@link Iterable} one. After that, we need to
* modify other parts of cactoos that relied on this in order to preserve their behaviour
* by using {@link Joined} and {@link Concatenated}.
*/
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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@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))

*/
public TextOf(final Iterable<Character> iterable) {
super(
new Joined(
", ",
"",
new Mapped<>(
Object::toString,
iterable
Expand All @@ -342,15 +340,7 @@ public TextOf(final Iterable<?> iterable) {
* @param iterator The iterable to convert to string
*/
public TextOf(final Iterator<Character> iterator) {
this(
new TextOfScalar(
() -> {
final StringBuilder buf = new StringBuilder();
iterator.forEachRemaining(buf::append);
return buf.toString();
}
)
);
this(new IterableOf<>(iterator));
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

}

/**
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/org/cactoos/map/MapEnvelopeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,18 @@ public void removeIsDelegated() {
).affirm();
}

@Test
public void testToString() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

@DmitryBarskov DmitryBarskov May 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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

new Assertion<>(
"Must be a readable representation",
new MapOf<String, Integer>(
new MapEntry<>("k1", 10),
new MapEntry<>("k2", -2)
).toString(),
new IsEqual<>("{k1=10, k2=-2}")
).affirm();
}

/**
* Class derived from MapEnvelope to use in some tests.
* @param <K> - key type
Expand Down
13 changes: 13 additions & 0 deletions src/test/java/org/cactoos/text/TextOfTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import java.nio.charset.StandardCharsets;
import org.cactoos.bytes.BytesOf;
import org.cactoos.io.InputOf;
import org.cactoos.iterable.IterableOf;
import org.cactoos.iterable.IterableOfChars;
import org.cactoos.iterator.IteratorOfChars;
import org.hamcrest.Matchers;
import org.hamcrest.core.IsEqual;
Expand Down Expand Up @@ -318,6 +320,17 @@ void printsStackTraceFromArray() {
).affirm();
}

@Test
void readsIterableToText() {
new Assertion<>(
"Must read Iterable to Text",
new TextOf(
new IterableOfChars("hello")
),
new IsText("hello")
).affirm();
}

@Test
void readsIteratorToText() throws Exception {
new Assertion<>(
Expand Down