Vavr: Iterator.toCharSeq() error

[ashrwin]

The following code fails

@Test
public void fail(){
    CharSeq seq = Iterator.of('a','b','c').toCharSeq();
    assertThat(seq.toString()).isEqualTo("abc");
}

with the following error

org.junit.ComparisonFailure: 
Expected :"abc"
Actual   :"Iterator(?)"
````
The problem is in this method
```java
//Value.java
default CharSeq toCharSeq() {
    return CharSeq.of(toString());
}

Here toString() calls AbstractIterator.toString()

[ashrwin]

Changing the toCharSeq() method to this works

@SuppressWarnings("unchecked")
default CharSeq toCharSeq() {
    return CharSeq.ofAll((Iterable<? extends Character>) iterator());
}

@danieldietrich Is there any better way of fixing this?

[danieldietrich]

Good catch!

Please do it this way:

interface Value<T> {

    default CharSeq toCharSeq() {
        if (this instanceof CharSeq) {
            return (CharSeq) this;
        } if (isEmpty()) {
            return CharSeq.empty();
        } else {
            return CharSeq.of(iterator().mkString());
        }
    }

}

Update: @ashrko619 I realized that the if (this instanceof Traversable) branch does not help very much - the overall complexity remains O(n). Therefore I removed it in the code above. Please consider it in your PR.

[danieldietrich]

@ashrko619 please do this in a separate PR - after it is fixed we can pull #1684

[ashrwin]

@danieldietrich There a lots of errors in the tests after fixing the toCharSeq method. For ex.

@Test
public void shouldConvertToCharSeq() {
    final Value<Integer> value = of(1, 2, 3);
    final CharSeq charSeq = value.toCharSeq();
    assertThat(charSeq).isEqualTo(CharSeq.of(value.toString()));
}

org.junit.ComparisonFailure: 
Expected :Vector(1, 2, 3)
Actual   :123

Should we create a new issue for fixing these tests?

[danieldietrich]

Mmhh, this will definitely change the semantics (and therefore won't be behavioral backward compatible any more) - but it is worth it. The current behavior can be seen as a bug.

Let's fix the tests in this issue. I think it should not be much work because several errors will depend on tests in an abstract test class. Fixing these tests will multiply regarding the specific collection tests.

If you think it is too much I'm able to do it this evening (approx. in 5 hours).

[ashrwin]

@danieldietrich No problems, I have already started working on fixing them 😉