The reasons why or why not \Magento\Framework\App\Action\Action::execute should be used as a plugin extension point

protected-execute-reasoning.md

To plug into or not plug into execute?

Background

Cyrill Schumacher asked in a tweet

Ur change to add abstract protected fnc exec() conflicts with the Plugin System or am I missing smthng?

This is about the protected visiblity of the method \Magento\Framework\App\Action\Action::execute().

abstract protected function execute();

Prior to the PR adding this method signature, it was not defined at all. It was simply called from \Magento\Framework\App\Action\Action::dispatch().

Reasoning

What was wrong with that? Too much for me to list all the reasons here. So I'll focus on the question "Why choose protected and not public?"

Reasons to make it public:

• It can be used as an extension point for plugins

Reasons to make it protected:

• Self-documenting code.
  There is a public method \Magento\Framework\App\ActionInterface::dispatch() defined on the interface all action classes have to implement.
  As a developer implementing an action class, or writing code that is a client to an action class, what do I have to implement? dispatch? execute? Both? What is the difference?
  The method names do not tell me.
  For me the goal to strive for is self-documenting code. It should be self-evident how a class is to be used just by looking at its interface. Having two public methods with similar names does not help this at all.
• Encapsulation.
  Making a method public but not adding it to the ActionInterface exposes a much bigger risk to change it in future. Even though officially all development should only be done against public methods defined in interfaces, I am pretty sure that folks will start writing plugins for any public methods that exist, even though they might change or go away in future.
  Changing them will surely become necessary, and when that happens third party developers with plugins on those methods will be unhappy.
  So from that point of view every method should have the most limited visibility that still allows it to do its work.
  Default should be private. If it is an abstract class exposing functionality, make it protected. Does that make it an extension point?
  Yes, but only for classes extending it. The external interface for the class can still be nice and orderly.
• Interface stability.
  Officially Magento 2 still is considered beta stability. But changing a method visibility is breaking backward compatibility.
  Since the visibility of execute wasn't even declared in Magento\Framework\App\Action\Action, using it as a plugin extension point certainly is asking for trouble.
  Declaring it as protected just makes that obvious, encouraging developers to look for a better extension point that is part of the ActionInterface, thus being stable.

To me, littering the code with public methods just to make it more plugin friendly is a recipe for technical dept. So the choice was to make it protected.

This discussion is moot

Besides the interesting arguments to be made about method visibility in general, this specific argument is a moot point. According to Anton Krill the execute method is going to replace the dispatch method (reference magento/magento2#1834 (comment)).
As such it will be public, togther with a signature change to accept the request object like dispatch does now.

Which goes to show that even using interface methods as extension points is dangerous during beta ;)