How far to go in replacing inner classes with lambdas?

[Remi Forax]

On 10/28/2012 05:53 PM, Joshua Bloch wrote:
> Brian,
>
> I would stop at the first refactoring: I'm not convinced that the 
> unbound method reference construct will /ever /look familiar. 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
>
> 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."

Brian,
you can go a step further because either r -> r.run() or Runnable::run 
are non capturing lambdas
thus they doesn't need to be stored in a static final field.
So you can replace

     public NotificationBroadcasterSupport(Executor executor, 
MBeanNotificationInfo... info) {
       this.executor = (executor != null) ? executor : defaultExecutor;
       ...
     }

by

public NotificationBroadcasterSupport(Executor executor, 
MBeanNotificationInfo... info) {
       // if the executor is null, the runnable is executed by the 
current thread
       this.executor = (executor != null) ? executor : r -> r.run();
       ...
     }

You can apply the same idea to j.u.functions.Mappers and Predicates.

cheers,
Rémi

>
> 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?
>
>