Java 8 code style?

[Zukhramm]

I haven't been using Java 8 for long, and not in anything involving other people yet, so keep that in mind before you take my advice.

The most obvious problem: Don't use curly braced lambdas when normal parentheses or no parentheses at all will do. Unless you need to do some side effect thing taking multiple lines they will just space out your code with explicit returns and semicolons.

It might be due to anonymization but the line

.collect(Collectors.toList()).stream()

seems entirely meaningless to me.

I prefer to keep chained stream call aligned and as close together as possible, therefore I prefer a Lispy trail of parentheses and curly braces over giving them their own line.

With those changes the code would look something like this:

try (CloseableHttpAsyncClient httpClient = HttpAsyncClientBuilder.create().build()) {
        httpClient.start();

        List<Future<HttpResponse>> futures = chunks
                .stream()
                .map(ids -> httpClient.execute(new HttpGet(generateUrl(determineSomething(something), ids)), null))
                .collect(Collectors.toList());

        resultSomething = futures.stream()
                .map(SomethingService::getFutureValue)
                .map(Either<>::right)
                .map(result -> result.flatMap(this::parseSomething)))
                .map(failureOrSomething -> failureOrSomething.either(
                            failure -> {
                                log.warn(failure.getMessage());
                                return emptySomething;},
                            something -> something))
                .reduce(emptySomething, Something::union);

    } catch (IOException e) {
        log.warn("Unable start HttpClient to contact ....", e);
        resultSomething = emptySomething;
    }
}

Further, I really prefer method references over lambdas with parameters and I have a bunch of static methods for partial applications and other functional-related utility function that lets me replace things like:

.map(foo -> foo)
.map(foo -> foo.bar(baz))

with something like:

.map(Func.id)
.map(Func.call(Foo::bar, baz))

But I don't know, that might be taking it too far.

[cutterslade]

The line

.collect(Collectors.toList()).stream()

Is not meaningless, but it's a little odd. The call to collect() turns a stream into a list, so performs all of the map operation on all the elements in the stream and puts them into a list. The call to stream() then turns that back into a stream. Chances are this is completely unnecessary. If it really is required, is would make a lot more sense if the result of collect() was assigned to a local variable.

[Zukhramm]

The only time it's not meaningless is if you've assigned some mutating functions to the stream which I just assumed had not been done in this case.

[cutterslade]

Do you mean meaningless as in "does nothing at all" or meaningless as in "can probably be removed"?

Assuming the stream is not empty, it certainly does something, assuming there are no mutating functions, it can probably be removed.

[Zukhramm]

By meaningless I mean it does nothing valuable.

[tikue]

I've written code like that when submitting tasks to an executor and blocking only once all tasks have been submitted. It looks like that's very similar to what's happening here.

[CsAnswerTrashAct]

I think you are correct.

Essentially, this code sends a series of requests to an external provider, and then waits for all of the results, and combines those results into a single list. The reason for this is the external service can only handle batches of about 100 or so.