What to do about weblib methods with lots of arguments
From MoodleDocs
Moodle 2.0
Note: This page is a work-in-progress. Feedback and suggested improvements are welcome. Please join the discussion on moodle.org or use the page comments.
In Moodle 2.0, all the functions in lib/weblib.php that are to do with generating output are being replaced by methods in lib/outputlib.php.
This is a chance to clean up the API. For example, we have already decided that all these methods will return a string, rather than giving a choice of outputting the HTML immediately via a $output parameter.
(Just to reassure you: We will make new versions of all the old weblib functions in deprecatedlib.php, that implement the old API in terms of the new methods, so you existing code will not break.)
The problem
While some of the weblib functions are quite sensible. For example:
function print_box($message, $classes='generalbox', $ids='', $return=false) {
some of them are ridiculous:
function choose_from_menu ($options, $name, $selected='', $nothing='choose', $script='',
$nothingvalue='0', $return=false, $disabled=false, $tabindex=0,
$id='', $listbox=false, $multiple=false, $class='') {
Suppose you want to create a <select> menu with a particular class attribute. You have to pass all 13 arguments, most of them just the default value, just to be able to pass class.
We should look for a better way to handle these methods with a lot of parameters.
Fortunately, there is a precedent. Here is the print_table function:
/**
* Print a nicely formatted table.
*
* @param array $table is an object with several properties.
* <ul>
* <li>$table->head - An array of heading names.
* <li>$table->align - An array of column alignments
* <li>$table->size - An array of column sizes
* <li>$table->wrap - An array of "nowrap"s or nothing
* <li>$table->data[] - An array of arrays containing the data.
* <li>$table->width - A percentage of the page
* <li>$table->tablealign - Align the whole table
* <li>$table->cellpadding - Padding on each cell
* <li>$table->cellspacing - Spacing between cells
* <li>$table->class - class attribute to put on the table
* <li>$table->id - id attribute to put on the table.
* <li>$table->rowclass[] - classes to add to particular rows. (space-separated string)
* <li>$table->colclass[] - classes to add to every cell in a particular colummn. (space-separated string)
* <li>$table->summary - Description of the contents for screen readers.
* <li>$table->headspan can be used to make a heading span multiple columns.
* <li>$table->rotateheaders - Causes the contents of the heading cells to be rotated 90 degrees.
* </ul>
* @param bool $return whether to return an output string or echo now
* @return boolean|string depending on $return
*/
function print_table($table, $return=false) {
That is, we just pass in one object which has all the necessary information as properties, many of which are optional. Should we use this approach for more functions?
On this page I explore this idea by rewriting the choose_from_menu function.
Simple example
Old code
A simple example (from backup/restore_form.html).
choose_from_menu($gradebook_history_options, "restore_gradebook_history",
$restore_gradebook_history, "");
New code
$menu = new moodle_select_menu;
$menu->options = $gradebook_history_options;
$menu->name = 'restore_gradebook_history';
$menu->selectedoption = $restore_gradebook_history;
echo $OUTPUT->select_menu($menu);
Complex example
Old code
A complex example (from mod/assignment/lib.php)
choose_from_menu($options, 'outcome_'.$n.'['.$userid.']',
$outcome->grades[$submission->userid]->grade, get_string('nooutcome', 'grades'),
'', 0, false, false, 0, 'menuoutcome_'.$n);
New code
$menu = new moodle_select_menu;
$menu->options = $options;
$menu->name = 'outcome_'.$n.'['.$userid.']';
$menu->selectedoption = $outcome->grades[$submission->userid]->grade;
$menu->nothinglabel = get_string('nooutcome', 'grades');
$menu->id = 'menuoutcome_'.$n;
echo $OUTPUT->select_menu($menu);
Although the new code is longer, I think it is much easier to see which parameters we have set (to non-default values).
Code behind the scenes
The remainder of this page is a bit out-of-date following some discussions which caused me to change some details above, however it is basically right.
Old code
The implementation of choose_from_menu is
/**
* Given an array of values, output the HTML for a select element with those options.
*
* Normally, you only need to use the first few parameters.
*
* @param array $options The options to offer. An array of the form
* $options[{value}] = {text displayed for that option};
* @param string $name the name of this form control, as in <select name="..." ...
* @param string $selected the option to select initially, default none.
* @param string $nothing The label for the 'nothing is selected' option. Defaults to get_string('choose').
* Set this to '' if you don't want a 'nothing is selected' option.
* @param string $script if not '', then this is added to the <select> element as an onchange handler.
* @param string $nothingvalue The value corresponding to the $nothing option. Defaults to 0.
* @param boolean $return if false (the default) the the output is printed directly, If true, the
* generated HTML is returned as a string.
* @param boolean $disabled if true, the select is generated in a disabled state. Default, false.
* @param int $tabindex if give, sets the tabindex attribute on the <select> element. Default none.
* @param string $id value to use for the id attribute of the <select> element. If none is given,
* then a suitable one is constructed.
* @param mixed $listbox if false, display as a dropdown menu. If true, display as a list box.
* By default, the list box will have a number of rows equal to min(10, count($options)), but if
* $listbox is an integer, that number is used for size instead.
* @param boolean $multiple if true, enable multiple selections, else only 1 item can be selected. Used
* when $listbox display is enabled
* @param string $class value to use for the class attribute of the <select> element. If none is given,
* then a suitable one is constructed.
* @return string|void If $return=true returns string, else echo's and returns void
*/
function choose_from_menu ($options, $name, $selected='', $nothing='choose', $script='',
$nothingvalue='0', $return=false, $disabled=false, $tabindex=0,
$id='', $listbox=false, $multiple=false, $class='') {
if ($nothing == 'choose') {
$nothing = get_string('choose') .'...';
}
$attributes = ($script) ? 'onchange="'. $script .'"' : '';
if ($disabled) {
$attributes .= ' disabled="disabled"';
}
if ($tabindex) {
$attributes .= ' tabindex="'.$tabindex.'"';
}
if ($id ==='') {
$id = 'menu'.$name;
// name may contaion [], which would make an invalid id. e.g. numeric question type editing form, assignment quickgrading
$id = str_replace('[', '', $id);
$id = str_replace(']', '', $id);
}
if ($class ==='') {
$class = 'menu'.$name;
// name may contaion [], which would make an invalid class. e.g. numeric question type editing form, assignment quickgrading
$class = str_replace('[', '', $class);
$class = str_replace(']', '', $class);
}
$class = 'select ' . $class; /// Add 'select' selector always
if ($listbox) {
if (is_integer($listbox)) {
$size = $listbox;
} else {
$numchoices = count($options);
if ($nothing) {
$numchoices += 1;
}
$size = min(10, $numchoices);
}
$attributes .= ' size="' . $size . '"';
if ($multiple) {
$attributes .= ' multiple="multiple"';
}
}
$output = '<select id="'. $id .'" class="'. $class .'" name="'. $name .'" '. $attributes .'>' . "\n";
if ($nothing) {
$output .= ' <option value="'. s($nothingvalue) .'"'. "\n";
if ($nothingvalue === $selected) {
$output .= ' selected="selected"';
}
$output .= '>'. $nothing .'</option>' . "\n";
}
if (!empty($options)) {
foreach ($options as $value => $label) {
$output .= ' <option value="'. s($value) .'"';
if ((string)$value == (string)$selected ||
(is_array($selected) && in_array($value, $selected))) {
$output .= ' selected="selected"';
}
if ($label === '') {
$output .= '>'. $value .'</option>' . "\n";
} else {
$output .= '>'. $label .'</option>' . "\n";
}
}
}
$output .= '</select>' . "\n";
if ($return) {
return $output;
} else {
echo $output;
}
}
New code
My new implementation of moodle_core_renderer::select_menu is
/**
* Output a <select> menu.
*
* You can either call this function with a single moodle_select_menu argument
* or, with a list of parameters, in which case those parameters are sent to
* the moodle_select_menu constructor.
*
* @param moodle_select_menu $selectmenu a moodle_select_menu that describes
* the select menu you want output.
* @return string the HTML for the <select>
*/
public function select_menu($selectmenu) {
if ($selectmenu->nothinglabel) {
$options = array($selectmenu->nothingvalue => $selectmenu->nothinglabel) +
$selectmenu->options;
} else {
$options = $selectmenu->options;
}
$attributes = array(
'name' => $selectmenu->name,
'id' => $selectmenu->id,
'class' => $selectmenu->get_classes_string(),
'onchange' => $selectmenu->script,
);
if ($selectmenu->disabled) {
$attributes['disabled'] = 'disabled';
}
if ($selectmenu->tabindex) {
$attributes['tabindex'] = $tabindex;
}
if ($selectmenu->listbox) {
if (is_integer($selectmenu->listbox)) {
$size = $selectmenu->listbox;
} else {
$size = min($selectmenu->maxautosize, count($options));
}
$attributes['size'] = $size;
if ($selectmenu->multiple) {
$attributes['multiple'] = 'multiple';
}
}
$html = $this->output_start_tag('select', $attributes) . "\n";
foreach ($options as $value => $label) {
$attributes = array('value' => $value);
if ((string)$value == (string)$selectmenu->selectedvalue ||
(is_array($selectmenu->selectedvalue) && in_array($value, $selectmenu->selectedvalue))) {
$attributes['selected'] = 'selected';
}
$html .= ' ' . $this->output_tag('option', $attributes, s($label)) . "\n";
}
$html .= $this->output_end_tag('select') . "\n";
return $html;
}
The moodle_select_menu class is little more than a stdClass. The main reason to have it is that it lets us PHPdoc the various fields. However note that moodle_core_renderer::select_menu will work fine if you pass a stdClass with the right fields.
/**
* This class hold all the information required to describe a <select> menu that
* will be printed by {@link moodle_core_renderer::select_menu()}. (Or by an overrides
* version of that method in a subclass.)
*
* All the fields that are not set by the constructor have sensible defaults, so
* you only need to set the properties where you want non-default behaviour.
*
* @copyright 2009 Tim Hunt
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @since Moodle 2.0
*/
class moodle_select_menu extends moodle_html_component {
/**
* @var array the choices to show in the menu. An array $value => $label.
*/
public $options;
/**
* @var string the name of this form control. That is, the name of the GET/POST
* variable that will be set if this select is submmitted as part of a form.
*/
public $name;
/**
* @var string the option to select initially. Should match one
* of the $options array keys. Default none.
*/
public $selectedvalue;
/**
* @var string The label for the 'nothing is selected' option.
* Defaults to get_string('choosedots').
* Set this to '' if you do not want a 'nothing is selected' option.
*/
public $nothinglabel = null;
/**
* @var string The value returned by the 'nothing is selected' option. Defaults to 0.
*/
public $nothingvalue = 0;
/**
* @var boolean set this to true if you want the control to appear disabled.
*/
public $disabled = false;
/**
* @var integer if non-zero, sets the tabindex attribute on the <select> element. Default 0.
*/
public $tabindex = 0;
/**
* @var mixed Defaults to false, which means display the select as a dropdown menu.
* If true, display this select as a list box whose size is chosen automatically.
* If an integer, display as list box of that size.
*/
public $listbox = false;
/**
* @var integer if you are using $listbox === true to get an automatically
* sized list box, the size of the list box will be the number of options,
* or this number, whichever is smaller.
*/
public $maxautosize = 10;
/**
* @var boolean if true, allow multiple selection. Only used if $listbox is true.
*/
public $multiple = false;
/**
* @deprecated
* @var string JavaScript to add as an onchange attribute. Do not use this.
* Use the YUI even library instead.
*/
public $script = '';
}
New support code
Note that the moodle_core_renderer::select_menu code depends on this code in moodle_renderer_base:
class moodle_renderer_base {
// ...
protected function output_tag($tagname, $attributes, $contents) {
return $this->output_start_tag($tagname, $attributes) . $contents .
$this->output_end_tag($tagname);
}
protected function output_start_tag($tagname, $attributes) {
return '<' . $tagname . $this->output_attributes($attributes) . '>';
}
protected function output_end_tag($tagname) {
return '</' . $tagname . '>';
}
protected function output_empty_tag($tagname, $attributes) {
return '<' . $tagname . $this->output_attributes($attributes) . ' />';
}
protected function output_attribute($name, $value) {
if ($value || is_numeric($value)) { // We want 0 to be output.
return ' ' . $name . '="' . $value . '"';
}
}
protected function output_attributes($attributes) {
$output = '';
foreach ($attributes as $name => $value) {
$output .= $this->output_attribute($name, $value);
}
return $output;
}
protected function output_class_attribute($classes) {
return $this->output_attribute('class', implode(' ', $classes));
}
}
