Cannot print inside forEach loop in stream java

Question

The method "combination" should make combination of input arrays. And I like to get this stream of combination and save it into a file.

public void writeDot() {
    try (PrintStream out = new PrintStream(path, "UTF-8")) {
        out.print("digraph {\n");
        String[] arr = {"hyo", "ji", "yoo", "mi", "vi", "se", "ari"};
        combination(arr, 2, 0, new String[2])
                .stream()
                .map(a -> Arrays.toString(a).join(" -> "))
                .forEach(out::print);
        out.println(";\n");
        out.println("}");
    } catch (IOException ioException) {
        ioException.printStackTrace();
    }
}

Combination methods goes like this:

public List<String[]> combination(String[] arr, int len, int startPosition, String[] result) {
    if (len == 0) {
        //System.out.println(Arrays.toString(result));
        return null;
    }
    for (int i = startPosition; i <= arr.length - len; i++) {
        result[result.length - len] = arr[i];
        combination(arr, len - 1, i + 1, result);
        list.add(result);
    }
    return list;
}

Result I expected is:

digraph {
hyo -> ji;
ji -> hyo;

and so on..
}

But I only get:

digraph {
;

}

What's wrong with my code? please help me.

Your `combination` is clearly wrong. You keep adding same instance to `list` list. — talex, Sep 07 '20 at 10:06
You should add a copy of the array to the result. `list.add(result);` → `list.add(result.clone());` — , Sep 07 '20 at 10:35
@HannahLee I think it's not the best idea to include the answer in the question. If you really feel that you found the answer yourself, you can post it as **answer**. Posting question and answer in the same post is confusing and not clear. — Giorgi Tsiklauri, Sep 07 '20 at 10:49

score 2 · Accepted Answer · edited Sep 07 '20 at 10:08

2

String.join is a static method that accepts two parameters: delimiter and array of elements.

You pass no elements, so result is empty string.

Correct code is:

combination(arr, 2, 0, new String[2])
                .stream()
                .map(a->String.join(" -> ", a))
                .forEach(out::print);

edited Sep 07 '20 at 10:08

Giorgi Tsiklauri

9,715
8
45
66

answered Sep 07 '20 at 10:02

talex

17,973
3
29
66

corsiKa · Answer 2 · 2020-09-07T10:20:23.097

0

It appears you have some issues with your combination method which are not helping your case because you're recursively calling it with only ever making one new result array. Instead, try a better combination method - ideally use a well tested library for this, like apache commons or guava.

public static List<String[]> makePairsFromArray(String[] arr) {
    List<String[]> list = new ArrayList<>();

    for(int i = 0; i < arr.length - 1; i++) {
        for(int j = i + 1; j < arr.length; j++) {
             String[] pair = new String[2];
             pair[0] = arr[i]; 
             pair[1] = arr[j];
             list.add(pair);
             String[] opp = new String[2];
             opp[0] = arr[j];
             opp[1] = arr[i];
             list.add(opp);
        }
    }
    return list;
}

Additionally, you will need to update the map(join) method call as described by talex.

edited Sep 07 '20 at 10:20

answered Sep 07 '20 at 10:02

corsiKa

81,495
25
153
204

You're correct, I had not realized the true intent of their code. It intially looked like they were making key value pairs. – corsiKa Sep 07 '20 at 10:07
Original `combine` method allow to choose size of arrays, your code only produce arrays with length = `2`. – talex Sep 07 '20 at 10:17
@talex yeah, it's quite clearly made for a digraph. Also, original code was fundamentally broken probably because it was cobbled together from different sites off google. – corsiKa Sep 07 '20 at 10:18

Cannot print inside forEach loop in stream java

2 Answers2