With more subsystems moving to OO code (speaking from my own "Field API as OO Plugins" sandbox), a lot of code gets moved from procedural functions in .module or .inc files to methods in class files.

As a typical gain from OO-ification, this helps grouping what was previously a flat list of functions (possibly grouped in .inc files if you took care of loading them properly) into logical groups, and reduces the amount of code that gets loaded on each page request.

One thing that is currently forced to stay in procedural land are FAPI and render API callbacks (#element_validate, #validate, #submit, #after_build, #pre_render, #post_render...), since the code that invokes them expects expects a func name in a flat string.
(another thing is theme hooks and (pre)processors, but I'll leave that out for another issue)

Those callbacks get orphaned in .module files while all the associated logic surrounding them has moved to a class file. Feels like a sore thumb, and mitigates the gains in terms of code organization and amount of loaded code.

No patch for now, but something like the following would allow methods to be used as callbacks :

function _form_validate(&$elements, &$form_state, $form_id = NULL) {
  [...]
       foreach ($elements['#element_validate'] as $function) {
-        $function($elements, $form_state, $form_state['complete_form']);
+        if (is_string($function)) {
+          $function($elements, $form_state, $form_state['complete_form']);
+        }
+        elseif (is_array($function)) {
+          $object = $function[0];
+          $method = $function[1];
+          if (is_object($object)) {
+            $object->$method($elements, $form_state, $form_state['complete_form']);
+          }
+          else {
+            // Static method in a class.
+            $object::$method($elements, $form_state, $form_state['complete_form']);
+          }
+        }
       }

calling code :

  $form['#element_validate'] = 'my_module_my_function';
  $form['#element_validate'] = array($this, 'myMethod');
  $form['#element_validate'] = array(get_class($this), 'myStaticMethod');

Maybe one thing to consider : the 2nd form de facto puts classed objects inside $form/$render structures. That might be something we'd rather not encourage, since $form and $render arrays tend to get serialized...
Then maybe limit to the 3rd form and static methods ? Dunno.

Also, I don't think this issue needs to be be postponed on a hypothetical "OO-ify Form API" effort.

Comments

yched’s picture

Issue summary: View changes

formatting error

yched’s picture

of course, the patch snippet above would need to be duplicated (factorized ?) in the places where the other #callbacks are invoked...

Michelle’s picture

I've been beating my head against this in D7. I realize it's not likely back-portable but just knowing it was going to be better in D8 would be wonderful. Sorry there's not much "added value" in this comment... I just wanted to do more than silently follow and say, yes, please, can we have this. :)

Michelle

Michelle’s picture

Issue summary: View changes

grammar

pounard’s picture

foreach ($elements['#element_validate'] as $callback) {
    if (is_callable($callback)) {
        $callback($elements, $form_state, $form_state['complete_form']);
    }

Should be enough to make it work. PHP 5.3 does not have the "callback" type hint, but it has everything else, my code would work. Althought it will work with PHP 5.2 too (not sure for array(object, methodname) thought).

Another viable syntax is:

$args = array($elements, $form_state, $form_state['complete_form']);
foreach ($elements['#element_validate'] as $callback) {
    if (is_callable($callback)) {
        call_user_func_array($callback, $args);
    }

But I'm quite sure the first would work, even if $callback is an array(object, methodname).

EDIT: In PHP callables can be:
- A function name
- An object implementing __invoke()
- A closure
- An array whose first value is an object, and second value is a method name
And there's probably some more. Just calling them as a function actually works by design except for array...

Actually the $callback() does not work with an array, but the call_user_func() and call_user_func_array() do.

yched’s picture

@pounard : most of these callbacks take their args by reference. That's why the current code uses $func(...) rather than c_u_f() / c_u_f_a().

pounard’s picture

Yes, true, I just read the PHP documentation. We can still put references into the $args array thought, it forces a call time reference but with no PHP warning and it works quite flawlessly.

plach’s picture

Allowing form builders and validate/submit handlers to be class methods is being addressed in #1499596: Introduce a basic entity form controller.

sun’s picture

$form['#element_validate'][] = array($this, 'validate');

One potential problem/concern with this is that we'd repetitively store $this many times within $form (and thus also in $form_state).

Cross-referencing #1499596: Introduce a basic entity form controller, which hit this problem as well (but only to limited extent).

pounard’s picture

Is this really a problem? It's a simple pointer.
EDIT: It will be as soon as the form gets in cache, I guess.

plach’s picture

@pounard:

It will be as soon as the form gets in cache, I guess.

Yes, I had some problem with this in the issue referenced above: the form controller is stored in the form state and the reference has to be updated after the form submission, otherwise changes to the form controller fields are not propagated to the form state.

pounard’s picture

Forms should probably not be cached, this would be the ultimate solution. There is not enough conceptual and pragmatic separation between the form object, the current form state, form processing handlers, and form values in Drupal.

plach’s picture

Or we could update the references automatically if the form is fetched form cache, provided that there is a reliable way to get the new reference. In the other issue there is.

pounard’s picture

This would work, but this would be complicated and just a dirty workarround for an API that reaches its own limits. I know that FAPI is complex and has a lot of history, and probably won't radicaly change today, so I guess it's the only solution for you.

yched’s picture

Yes, see the end of the OP. Storing classed objects within $form is problematic.
Adds a lot of overhead on the size of the cached serialized $form, and objects with closures will simply break (and we're still not too sure on how the DICs will leak into various objects)

In my "Field API as plugins" branch, I hacked form.inc a bit to allow methods as #validate callbacks, but only static methods - thus, what you put in $form['#validate'] is only array('className', 'methodName'). A bit more restrictive, but then again a FAPI callback is currently supposed to receive all its params through the $form / $form_state arguments, so forbidding FAPI "methods as callbacks" to depend on the state of an instanciated object is kind of in line with what we currently have...

plach’s picture

Well, explictly forbidding them would be problematic for entity forms where a workaround exists.

sun’s picture

The form cache is required for forms that are very complex/resource-intensive to construct, and also for stuff like Ajax.

However, it is somewhat true that we're slightly abusing the form cache for entity forms currently. These forms actually want to cache the $form_state only, without the therein contained 'complete_form' (i.e., $form).

Technically, for the scope of this issue, I'd think we'd want to store a pointer to a classed object in the $form_state. How about this?

  $form_state['classes']['field_instance_field_body_123'] = $plugin_instance;
...
  $form['#element_validate'][] = array($plugin_instance->getID(), 'validate');
...
...
  foreach ($element['#element_validate'] as $callback) {
    if (is_array($callback)) {
      list($class_id, $method) = $callback;
      $form_state['classes'][$class_id]->$method($element, $form_state);
    }
    else {
      $callback($element, $form_state);
    }
  }
fago’s picture

One potential problem/concern with this is that we'd repetitively store $this many times within $form (and thus also in $form_state).

True. Serialize() is intelligent enough to handle object identities properly though, however that only works for arrays that are serialized at whole. Thus, with having the same object in $form and $form_state we would still end up with two serializations of the object with the current split form and form-state cache. If we'd combine both caches that shouldn't be a problem any more though.

Still, I dislike having all the objects referenced in $form. All form-related objects and data belongs into the form state.

list($class_id, $method) = $callback;
$form_state['classes'][$class_id]->$method($element, $form_state);

Interesting idea. What about using a parents array + drupal_array_get_nested_value() for navigating through the form state? So one could use meaningful object locations in form state instead?
E.g.

list($parents, $method) = $callback;
$object = drupal_array_get_nested_value($form, $parents);
$object->$method($element, $form_state);

Going further, I really like having form controller classes for implementing forms in the first place, similar to what #1499596: Introduce a basic entity form controller is coming up with. So, the controllers methods could be the default #validate and #submit, instead of magic callback detection. If we'd enforce that and encourage class base form controllers, we could even do away with form specific custom including, what would remove a huge pain point in re-using forms... (well that belongs into another issue, so stopping here...)

plach’s picture

Issue tags: +Entity forms

Marking as follwup of #1499596: Introduce a basic entity form controller: we need to ensure that all the FAPI callbacks can accept object methods. Better: any callable, for consistency.

sun’s picture

sun’s picture

Issue summary: View changes

Doh.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

Version: 8.9.x-dev » 9.2.x-dev

It looks outdated

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.