How far to go in replacing inner classes with lambdas?

[Brian Goetz]

> I would stop at the first refactoring

Indeed, I think that's a pretty sensible place to stop *in this case* 
(at least in 2012; how we feel about this code in 2017 might well be 
different after we've been using these features for a few years.)  The 
reason I bring this up is that there are not yet well-understood 
guidelines about when and how best to use these new features, but the 
earlier we can come up with proto-EJ-like material, the better chance we 
have of encouraging people to use these features right.  There are a lot 
of cases where you CAN but its not clear you SHOULD.  Many programmers 
will tend to erroneously think that the smaller code is necessarily 
better, and especially when the IDE is goading them on to do so, may 
take it too far.  On the other hand, unbound method refs are sometimes 
exactly what the doctor ordered.

But, this doesn't really answer my question yet.  In this example, we 
had a pretty clear intuition about what the right degree was.  But, 
different cases will be different, and there are plenty of cases where 
the method ref is arguably *more* clear than the lambda.  So, what I was 
trying to get at was: what guidelines can we come up with for when we 
recommend one vs the other?

> I'm not convinced that the
> unbound method reference construct will /ever /look familiar.

I think in cases where it is obvious what the semantics of the target 
type are, it will become familiar quite quickly:

   Comparator<Person> c = Comparators.comparing(Person::getLastName);

or

   shapes.stream().filter(Shape::isBlue).into(blueShapes);

I don't think anyone who understands what filter() or comparing() does 
(filter is obvious; comparing takes a function which extracts a 
Comparable key from a type T, and returns a Comparator<T>) will have any 
trouble getting used to this construct.  What is most confusing about 
this example is the higher-order-function-ness of comparing(), but once 
you get past that, its pretty clear, and I would argue that both of 
these examples are more readable than the lambda alternatives:

   Comparator<Person> c = Comparators.comparing(p -> p.getLastName());

   shapes.stream().filter(s -> s.isBlue()).into(blueShapes);

I think what makes these different from the Executor example is that it 
is not completely obvious from context what an Executor does, so you 
have to go and look at the definition of Executor and reason through 
what is happening, which is less intuitive.

I don't expect anyone to have a candidate rule off the top of their 
head.  But, I think it would be extremely useful if we could offer some 
guidelines on when different approaches are warranted.

> That said,
> neither of the refactorings is as easy to understand as the original
> code declaration. In the first, you loose the fact that the argument to
> an Executor's sole abstract method is of type Runnable. When you're
> dealing with two SAM types at once (Executor and Runnable in this case),
> things can get a bit confusing. The second refactoring does make the
> argument type (Runnable) explicit, but it's just unreadable.
>
> A casualty in both refactorings is the comment:
>
>              // DirectExecutor using caller thread

Yes, I had actually intended to retain the comment in the refactored 
version, but got distracted while writing my e-mail.

> It's invaluable in keeping the code readable, and must be maintained.
>
>      Josh
>
> P.S. While you're cleaning up the code, "final static" should be "static
> final."
>
> On Sun, Oct 28, 2012 at 9:12 AM, Brian Goetz <brian.goetz at oracle.com
> <mailto:brian.goetz at oracle.com>> wrote:
>
>     The IntelliJ EAP builds already have inspections for "replace inner
>     with lambda" and "replace lambda with method reference" (nice job,
>     guys!)
>
>     In browsing through the inspection results in the JDK, we find
>     ourselves in the same position we did with Coin features like
>     Diamond; many code snippets can be simplified, but not all code
>     snippets that can be simplified should be.
>
>     Here's an example:
>
>          private final static Executor defaultExecutor = new Executor() {
>                  // DirectExecutor using caller thread
>                  public void execute(Runnable r) {
>                      r.run();
>                  }
>              };
>
>     We could simplify this to:
>
>          private final static Executor defaultExecutor = r -> r.run();
>
>     or further to
>
>          private final static Executor defaultExecutor = Runnable::run;
>
>     (Executor is a SAM interface with one method, execute(Runnable).)
>
>
>     What guidelines can we offer users on when they should and should
>     not replace inner classes with lambdas or method reference?  Does
>     this last one look a little weird only because the powerful "unbound
>     method reference" construct is still a little unfamiliar?
>
>