-1

I've got a String[] with values that need to be put into a String[][] with each "row" consisting of the current index and the value of the former array. An iterative solution looks like this:

String[] vals = getValsFromSomewhere();
String[][] targetArray = new String[vals.length][];
for (int i = 0; i < targetArray.length; i++) {
    targetArray[i] = new String[]{
        Integer.toString(i), vals[i]
    };
}

I currently try to change that to streams to avoid temporary variables and get rid off for-loops throughout my method but the best solution I came up with so far is:

String[] vals = getValsFromSomewhere();
String[][] targetArray = IntStream.range(0, vals.length)
    .mapToObj(index -> new String[]{
        Integer.toString(index), vals[index]
     })
    .toArray(String[][]::new);

It's a solution with streams, all right. But part of my original goals, to get rid off temporary variables (vals in this case), isn't achieved. Searching through available methods in Stream hasn't brought up anything that looks useful but I might have missed something. That's why I'm asking here ;-) Pointing me to the right direction is highly appreciated.

shmosel
  • 49,289
  • 6
  • 73
  • 138
Lothar
  • 5,323
  • 1
  • 11
  • 27
  • 4
    You got it right, and haven't missed anything. But really, why use a String array to store an integer and a String? Why don't you use a class with two correctly typed properties? – JB Nizet Oct 25 '17 at 15:43
  • Because that's what the subsequent API-call requires as parameter. – Lothar Oct 25 '17 at 15:45
  • 4
    Oh my. Condolences. – JB Nizet Oct 25 '17 at 15:47
  • @JBNizet Not really and there would be still the same problem with a dedicated class, because you're still left need to get its corresponding value to set. – Lothar Oct 26 '17 at 17:30

2 Answers2

0

Removing the temporary variable vals means you can't know how many elements are to be processed before creating the Stream, so using range() is not possible. I can't see how to eliminate all temporary variables, but as an alternative I introduced an AtomicInteger that gets incremented within the stream processing to support the indexing.

I'm certainly not claiming that as a better approach than yours, and I'd be interested if there are any other approaches for indexing with streams apart from range() and AtomicInteger (using only standard Java).

Here's the code:

String[] getValsFromSomewhere() {

    return new String[]{"The", "rain", "in", "Spain", "stays", "mainly", "in", "the", "plain!"};
}

void test() {

    AtomicInteger ai = new AtomicInteger(0);

    String[][] target = Stream.of(getValsFromSomewhere())
                              .map(e -> new String[]{e, String.valueOf(ai.getAndIncrement())})
                              .toArray(String[][]::new);

    for(int row=0; row<target.length; row++) {
        for(int element=0; element<target[row].length; element++) {
            System.out.println("target[" + row +"] [" + element + "] = " + target[row][element]);
        }
    }    
}
skomisa
  • 16,436
  • 7
  • 61
  • 102
  • Thanks for that idea. You eliminated one temporary variable by introducing a new one, so with this I'm still in the same situation as before (that is: having temporary variables where I don't need them) just with code that others might confuse what actually happens and why. And I don't want to be featured at thedailywtf ;-) – Lothar Oct 26 '17 at 09:00
  • @Lothar Understood, and I realize that you don't want any temporary variables, but whether you "don't need them" is another matter. I suspect that for your requirement a temporary variable may be essential, but I'll be happy to be proven wrong. – skomisa Oct 26 '17 at 14:09
0

Why not write your own Collector (or better said let me write one for you)? Seems maybe a bit overkill but that's what you could use:

class Array2DCollector implements Collector<String, Map<Integer,String>, String[][]>{

    private final AtomicInteger index = new AtomicInteger(0);

    @Override
    public Supplier<Map<Integer, String>> supplier(){
        return HashMap::new;
    }

    @Override
    public BiConsumer<Map<Integer, String>, String> accumulator(){
        return (map, string) -> map.put(index.getAndIncrement(), string);
    }

    @Override
    public BinaryOperator<Map<Integer, String>> combiner(){
        return (map1, map2) -> {
            map1.putAll(map2);
            return map1;
        };
    }

    @Override
    public Function<Map<Integer, String>, String[][]> finisher(){
        return map -> {
            final String[][] array = new String[index.get()][2];
            for(int i = 0; i < map.size(); i++){
                array[i][0] = String.valueOf(i);
                array[i][1] = map.get(i);
            }
            return array;
        };
    }

    @Override
    public Set<Characteristics> characteristics(){
        return EnumSet.noneOf(Characteristics.class);
    }
}

Which then can easily be used as follows

final String[][] target = Arrays.stream(vals).collect(new Array2DCollector());

Explanation: short in short, the collector has an internal variable index which is just for counting. For every element in the stream, this variable gets increased, and the element gets saved in the hashMap. At last the hashmap is converted to a 2D-Array.

Important Note: This collector is based on HashMap, it cannot be executed Concurrently and is Ordered.

Final Note: As said this is a bit overkill, but why not give it a try ;)

Lino
  • 19,604
  • 6
  • 47
  • 65
  • Your implementation works fine, but creating a Collector that is designed only to work for sequential streams is not good, since there is nothing to prevent a caller from using that Collector with a parallel stream. To address that, remove the code from the combiner() method, and return null instead. Processing a sequential stream will still work, but processing a parallel stream will fail with a NullPointerException, which is much better than potentially returning garbage results without any error. – skomisa Oct 27 '17 at 22:04
  • @skomisa you got a point, but why not just throw directly, so you can give a clear error, rather than returning null – Lino Oct 29 '17 at 14:21
  • The problem with that approach is that you cannot assume that the combiner() method will not be called for a sequential stream, and in the case of your code it is. (Add a println() to combiner() to verify that yourself.) So if combiner() blindly throws an exception it will break the sequential stream as well as the parallel stream. It seems that the value returned by your combiner() is discarded when processing a sequential stream. See https://stackoverflow.com/questions/29210176/can-a-collectors-combiner-function-ever-be-used-on-sequential-streams – skomisa Oct 29 '17 at 15:33
  • 1
    But the larger point is that if the code is concerned with whether the stream is sequential or parallel then the overall approach probably needs rethinking. – skomisa Oct 29 '17 at 15:36
  • Thanks for that. I will give it a try and I think I will learn something new about streams, then ;-) – Lothar Oct 29 '17 at 19:05