Need some help thinking in lambdas from my fellow StackOverflow luminaries.
Standard case of picking through a list of a list of a list to collect some children deep in a graph. What awesome ways could Lambdas
help with this boilerplate?
public List<ContextInfo> list() {
final List<ContextInfo> list = new ArrayList<ContextInfo>();
final StandardServer server = getServer();
for (final Service service : server.findServices()) {
if (service.getContainer() instanceof Engine) {
final Engine engine = (Engine) service.getContainer();
for (final Container possibleHost : engine.findChildren()) {
if (possibleHost instanceof Host) {
final Host host = (Host) possibleHost;
for (final Container possibleContext : host.findChildren()) {
if (possibleContext instanceof Context) {
final Context context = (Context) possibleContext;
// copy to another object -- not the important part
final ContextInfo info = new ContextInfo(context.getPath());
info.setThisPart(context.getThisPart());
info.setNotImportant(context.getNotImportant());
list.add(info);
}
}
}
}
}
}
return list;
}
Note the list itself is going to the client as JSON
, so don't focus on what is returned. Must be a few neat ways I can cut down the loops.
Interested to see what my fellow experts create. Multiple approaches encouraged.
EDIT
The findServices
and the two findChildren
methods return arrays
EDIT - BONUS CHALLENGE
The "not important part" did turn out to be important. I actually need to copy a value only available in the host
instance. This seems to ruin all the beautiful examples. How would one carry state forward?
final ContextInfo info = new ContextInfo(context.getPath());
info.setHostname(host.getName()); // The Bonus Challenge
It's fairly deeply nested but it doesn't seem exceptionally difficult.
The first observation is that if a for-loop translates into a stream, nested for-loops can be "flattened" into a single stream using
flatMap
. This operation takes a single element and returns an arbitrary number elements in a stream. I looked up and found thatStandardServer.findServices()
returns an array ofService
so we turn this into a stream usingArrays.stream()
. (I make similar assumptions forEngine.findChildren()
andHost.findChildren()
.Next, the logic within each loop does an
instanceof
check and a cast. This can be modeled using streams as afilter
operation to do theinstanceof
followed by amap
operation that simply casts and returns the same reference. This is actually a no-op but it lets the static typing system convert aStream<Container>
to aStream<Host>
for example.Applying these transformations to the nested loops, we get the following:
But wait, there's more.
The final
forEach
operation is a slightly more complicatedmap
operation that converts aContext
into aContextInfo
. Furthermore, these are just collected into aList
so we can use collectors to do this instead of creating and empty list up front and then populating it. Applying these refactorings results in the following:I usually try to avoid multi-line lambdas (such as in the final
map
operation) so I'd refactor it into a little helper method that takes aContext
and returns aContextInfo
. This doesn't shorten the code at all, but I think it does make it clearer.UPDATE
But wait, there's still more.
Let's extract the call to
service.getContainer()
into its own pipeline element:This exposes the repetition of filtering on
instanceof
followed by a mapping with a cast. This is done three times in total. It seems likely that other code is going to need to do similar things, so it would be nice to extract this bit of logic into a helper method. The problem is thatfilter
can change the number of elements in the stream (dropping ones that don't match) but it can't change their types. Andmap
can change the types of elements, but it can't change their number. Can something change both the number and types? Yes, it's our old friendflatMap
again! So our helper method needs to take an element and return a stream of elements of a different type. That return stream will contain a single casted element (if it matches) or it will be empty (if it doesn't match). The helper function would look like this:(This is loosely based on C#'s
OfType
construct mentioned in some of the comments.)While we're at it, let's extract a method to create a
ContextInfo
:After these extractions, the pipeline looks like this:
Nicer, I think, and we've removed the dreaded multi-line statement lambda.
UPDATE: BONUS CHALLENGE
Once again,
flatMap
is your friend. Take the tail of the stream and migrate it into the lastflatMap
before the tail. That way thehost
variable is still in scope, and you can pass it to amakeContextInfo
helper method that's been modified to takehost
as well.First attempt beyond ugly. It will be years before I find this readable. Has to be a better way.
Note the
findChildren
methods return arrays which of course work withfor (N n: array)
syntax, but not with the newIterable.forEach
method. Had to wrap them withArrays.asList
The utility methods
And finally a Lambda-powerable implementation of
Iterable
Lots of plumbing and no doubt 5x the byte code. Must be a better way.
This would be my version of your code using JDK 8 streams, method references and lambda expressions:
In this approach I replace your if-statements for filter predicates. Take into account that an
instanceof
check can be replaced with aPredicate<T>
which can also be expressed as
Similarly, your casts can be replaced by
Function<T,R>
.Which is pretty much the same as
And adding items manually to a list in the for loop can be replaced with a collector. In production code, the lambda that transforms a
Context
into aContextInfo
can (and should) be extracted into a separate method, and used as a method reference.Solution to bonus challenge
Inspired by @EdwinDalorzo answer.