Aryeh Gregor wrote:
So I'm thinking that the general tactic should be 1) have special parameters for anything that's required or practically always used, but 2) relegate everything else to the $attribs array. So if we had a hypothetical Html::a(), it would look like:
public static function a( $href, $attribs = array() ) {
and img might be:
public static function img( $src, $alt, $attribs = array() ) {
I'm afraid your thinking on this is going in precisely the opposite direction to mine. I think long lists of formal parameters make code almost impossible to read and nearly as difficult to write. It might take slightly longer to type
Html::img( array( 'src' => $src, 'alt' => $alt ) );
but at least you can read that, and write it, without having to look up the parameter order in the documentation, and without having to memorise the parameter orders for large numbers of similar functions.
Unless a programmer uses a function regularly, recalling what order the parameters should be in will be a difficult task, requiring several seconds if it's possible at all.
I think code should be readable. In cases where that means typing more characters, I always give people the same flippant response: learn to type faster. But typing is generally a fast process, and uses up a small percentage of the time required for software development, especially when compared to review, testing and debugging.
Another problem with formal parameters is that it's hard to know when to stop. When there are a lot of them, with most of them optional and rarely used, it makes sense to pass an array instead. But people have no problem with adding "just one more" parameter to support their desired feature, so the list becomes bloated. When the time comes to merge the optional parameters into a single array, backwards compatibility is difficult. If you start with a parameter array from the outset, growth in functionality is not a problem and there's never a need to break the interface.
For instance, Article::insertNewArticle() started like this:
function insertNewArticle( $text, $summary, $isminor, $watchthis )
And it grew to this:
function insertNewArticle( $text, $summary, $isminor, $watchthis, $suppressRC=false, $comment=false, $bot=false )
I tried to reduce the length of the parameter list by moving UI-related code to the caller, resulting in this:
function doEdit( $text, $summary, $flags = 0 )
But despite room for growth in the $flags parameter, before long it grew to this:
function doEdit( $text, $summary, $flags = 0, $baseRevId = false, $user = null )
Linker::makeKnownLinkObj() started like this:
function makeKnownLinkObj( $title, $text = "", $query = "", $trail = "" )
And grew to this:
function makeKnownLinkObj( $title, $text = '', $query = '', $trail = '', $prefix = '' , $aprops = '', $style = '' )
Before being gradually replaced by this starting in MW 1.14:
function link( $target, $text = null, $customAttribs = array(), $query = array(), $options = array() )
which is an unfortunate example of bad design from the outset, since there are 5 formal parameters, 4 of them optional, and a number of callers only override $options. Consider this:
$sk->link( $target, null, array(), array(), array( 'known' ) );
versus this:
$sk->link( $target, array( 'known' => true ) );
and tell me which one is easier to understand and type.
I've found that a coding style leaning heavily towards array parameters is effective and flexible, allowing rapid development without having to worry about potential future growth. I've been heading in this direction in my own projects in both extensions and core, even going as far as converting wfFindFile() and some associated functions to a parameter array style in r55082. That felt so good that I'd like to try it with some other core functions with bloated parameter lists.
It would be good if we could have some sort of consensus on this so that we don't end up converting each others' code.
-- Tim Starling