I have the following code:
IntStream.range(0, width).forEach(x1 -> {
IntStream.range(0, height).forEach(y1 -> {
IntStream.rangeClosed(x1-1, x1+1).forEach(x2 -> {
IntStream.rangeClosed(y1-1, y1+1).forEach(y2 -> {
if ((x1 != x2 || y1 != y2) && getNode(x2, y2) != null){
getNode(x1, y1).registerObserverAtNeighbor(getNode(x2, y2));
}
});
});
});
});
Is there a way to write the above using fewer nested statements? It's basically "for each node from (0,0) to (width,height) register observer at nodes from (x-1,y-1) to (x+1,y+1), but not at self".
In principle, you can replace nested loops with a Stream
using flatMap
. This requires that you choose an element type capable of holding the information equivalent to the loop variables, if you need it. In your case, it’s the two values for x
and y
. It would simplify the code if your Node
class holds these information as you can easily iterate over nodes rather than int
values then. Since you didn’t specify the capabilities of your Node
class, here is an example which uses a long[]
of size two to hold the points:
IntStream.range(0, width).boxed()
.flatMap(x->IntStream.range(0, height).mapToObj(y->new int[]{ x, y }))
.forEach(p1 -> {
Consumer<Node> register=getNode(p1[0], p1[1])::registerObserverAtNeighbor;
IntStream.rangeClosed(p1[0]-1, p1[0]+1).boxed()
.flatMap(x->IntStream.rangeClosed(p1[1]-1, p1[1]+1).mapToObj(y->new int[]{ x,y }))
.filter(p2 -> (p1[0] != p2[0] || p1[1] != p2[1]))
.map(point -> getNode(point[0], point[1]))
.filter(node -> node != null)
.forEach(register);
});
It still simplifies the innermost code by moving code to the outside, where possible, e.g. the getNode
calls. You can also simplify the code by putting the repeated task of creating a Stream
over points for an area into a method:
static Stream<int[]> area(int x0, int x1, int y0, int y1) {
return IntStream.range(x0, x1).boxed()
.flatMap(x->IntStream.range(y0, y1).mapToObj(y->new int[]{ x, y }));
}
Then you can use it like this:
area(0, width, 0, height).forEach(p1 -> {
Consumer<Node> register=getNode(p1[0], p1[1])::registerObserverAtNeighbor;
area(p1[0]-1, p1[0]+2, p1[1]-1, p1[1]+2)
.filter(p2 -> (p1[0] != p2[0] || p1[1] != p2[1]))
.map(point -> getNode(point[0], point[1]))
.filter(node -> node != null)
.forEach(register);
});
It still could be easier if you have/use a dedicated point class or if the node class holds the point information (and has a comparison method for it, in the best case).
What you have is basically 4 nested loops. It makes sense because you are iterating over two dimensions of a matrix, and then, for each node, you iterate over a small matrix that consists of its neighbourgs.
Something like this.
0000000
0---000
0-X-000
0---000
0000000
I guess you could use a recursive function just for the syntax, although no benefit really.
iterateLambdas(0, width, 0, height, 1);
public static void iterateLambdas(
int start1,
int end1,
int start2,
int end2,
int depth) {
IntStream.range(start1, end1).forEach(x1 -> {
IntStream.range(start2, end2).forEach(y1 -> {
if (depth != 0) {
iterateLambdas(x1 - 1, x1 + 2, y1 - 1, y1 + 2, depth - 1);
} else {
// Current node : (start1 + 1), (start2 + 1)
// Current neighbour : x1, y1);
// Your logic here
}
});
});
}
Since you operate on nodes I suggest to create streams of nodes in the first place. Please note that I'm making some assumptions about the nodes.
getNodes(0, width - 1, 0, height - 1).forEach(node -> {
getNodes(node.getX() - 1, node.getX() + 1, node.getY() - 1, node.getY() + 1)
.filter(neighbor -> !neighbor.equals(node))
.forEach(neighbor -> node.registerObserverAtNeighbor(neighbor));
});
Creating a stream using your approach:
private static Stream<Node> getNodes(int x1, int x2, int y1, int y2) {
return IntStream.rangeClosed(x1, x2)
.mapToObj(x -> (Stream<Node>)IntStream.rangeClosed(y1, y2).mapToObj(y -> getNode(x, y)))
.flatMap(nodes -> nodes)
.filter(node -> node != null);
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With