Okay, so I made this nice new Html class lately to mostly replace Xml for HTML output. One of the major purposes of Xml, and now one of the major purposes of Html, is to provide convenient wrapper functions for particular elements that are more concise than Html::element( 'foo', array( 'x' => 'y', 'z' => 'w' ) ). Here's what the input() method currently looks like:
public static function input( $name, $value = null, $type = 'text', $attribs = array() ) {
There are three "special" attributes: name, value, and type. In the Xml version, the special attributes are instead name, size, and value. The problem with this is that 1) none of these three are actually required, and at least two of the three frequently just use the default; and 2) the order isn't obvious or easy to remember. Moreover, in practice, the $attribs are usually specified, so the parameter defaults don't help much. If we have "convenience" shortcuts like this for many other elements, they'll indeed be more concise, but hard to actually understand or use.
So we could do this . . .
public static function input( $attribs = array() ) {
. . . but that's not really convenient at all.
So one way that Xml handles this is to just have even more specialized functions, like:
public static function hidden( $name, $value, $attribs = array() ) {
The nice thing here is that there's practically no reason you'd ever want to make a hidden input without both name and value specified, but often those are the only things you want to specify. So often you can replace Html::element( 'input', array( 'type' => 'hidden', 'name' => 'myName', 'value' => 'foo' ) ) with just Html::hidden( 'myName', 'foo' ), and that's pretty clear, since hidden inputs logically have two things they almost always need to have specified, and there's an obvious order (key then value).
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() ) {
On the other hand, this would mean you'd have to do something like
public static function input( $name, $attribs = array() ) {
which is fairly verbose in practice. Of course, as noted, you normally need to use $attribs anyway for input, so it's not really that much worse than necessary.
What are other people's thoughts on what a nice interface would look like here? (Other than "PHP should have named parameters like Python"? :) ) Xml is very inconsistent on this score, and it would be nice if we had a consistent format for Html.
On 22/08/2009, at 12:06 AM, Aryeh Gregor wrote:
public static function a( $href, $attribs = array() ) {
Is the omission of a content parameter deliberate?
-- Andrew Garrett agarrett@wikimedia.org http://werdn.us/
On Sat, Aug 22, 2009 at 8:56 AM, Andrew Garrettagarrett@wikimedia.org wrote:
On 22/08/2009, at 12:06 AM, Aryeh Gregor wrote:
public static function a( $href, $attribs = array() ) {
Is the omission of a content parameter deliberate?
No, I was just thoughtlessly copying from input. I'm not sure a() would be very useful anyway.
Actually, I think input() and friends could mostly be replaced with HtmlForm, if that does what I think it does (I haven't actually tried to use it). So maybe I should start porting things to HtmlForm instead of Html::input(). That would give us all sorts of transparent input validation and nice things like that.
On 8/21/09 4:06 PM, Aryeh Gregor wrote:
What are other people's thoughts on what a nice interface would look like here? (Other than "PHP should have named parameters like Python"? :) ) Xml is very inconsistent on this score, and it would be nice if we had a consistent format for Html.
Most of the Xml:: wrappers are about as consistent as I think you can reasonably get without giving everything the same form as not having defaults, overall. :)
The Xml:: and Html:: functions are fairly low-level; we've also now got the HtmlForm class, which Andrew built for the Special:Preferences rewrite.
This gives you a way to build a form descriptively, including specifying input validation and giving a clean way for extensions to add things to your form...
My inclination is that most UI code should be using HtmlForm or something like it to define its form structure, and not be building forms from scratch (whether out of raw HTML strings or building them element-by-element with helper functions).
-- brion
On Sat, Aug 22, 2009 at 10:32 PM, Brion Vibberbrion@wikimedia.org wrote:
My inclination is that most UI code should be using HtmlForm or something like it to define its form structure, and not be building forms from scratch (whether out of raw HTML strings or building them element-by-element with helper functions).
That's my thought too, on reflection. Most of the helper functions in Xml:: end up being form-related. Let me try to port some stuff over; there's still time to remove Html::input() and Html::hidden() entirely.
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
On Mon, Aug 24, 2009 at 11:24 AM, Tim Starlingtstarling@wikimedia.org wrote:
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.
. . .
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.
I agree with pretty much everything you've said. I only wish PHP supported named parameters like Python, since PHP's array syntax is pretty ugly. I'll see about removing Html::input(), at least. I'm not sure yet whether HTMLForm is a suitable replacement, since it has essentially no comments and I haven't yet done enough experimentation to figure out how it works . . .
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.
To be fair, $sk->knownLink( $target ) is easier to type than either and no harder to understand, and that's what's used now for this *specific* case. I agree that condensing the options is a good idea, though -- it's rare that any caller uses all five parameters, but many use at least two or three, and often not the first two or three. Something like
$sk->link( $this->mTitle, array( 'text' => wfMsgHtml( 'markaspatrolledtext' ), 'query' => array( 'action' => 'markpatrolled', 'rcid' => $rcid ), 'known', 'noclasses' ) )
isn't very pretty, but it's certainly much more readable. I can't remember the order of the parameters myself, and I wrote them.
It's actually pretty easy in PHP to totally switch over the kind of parameters that a function takes. In the case of link(), we could collapse the last four arguments into an associative array, and use func_get_args() to fall back to the old way if the second argument is a non-array.
Tim Starling wrote:
Aryeh Gregor wrote:
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 ) );
...although at that point you might almost as well just go the extra step and make it:
Html::element( 'img', array( 'src' => $src, 'alt' => $alt ) );
On Tue, Aug 25, 2009 at 8:33 AM, Ilmari Karonennospam@vyznev.net wrote:
Tim Starling wrote:
Aryeh Gregor wrote:
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 ) );
...although at that point you might almost as well just go the extra step and make it:
Html::element( 'img', array( 'src' => $src, 'alt' => $alt ) );
-- Ilmari Karonen
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I'm a fan of using the arrays for the majority of params, how Tim describes. Classes, IDs, default values, all of this stuff should be in an associative array.
The only exception I can think of is for the absurdly obvious ones: Html::a( $href, array() ) or Html::img( $src, array() ); To me, putting the tags in that form would be the most intuitive.
-Chad
Ilmari Karonen wrote:
Tim Starling wrote:
Html::img( array( 'src' => $src, 'alt' => $alt ) );
...although at that point you might almost as well just go the extra step and make it:
Html::element( 'img', array( 'src' => $src, 'alt' => $alt ) );
Why not go full way:
Html::doStuff( array( 'what' => 'element', 'element' => 'img', 'src' => $src, 'alt' => $alt ) );
Seriously, I see a logical distinction between obligatory (every image must have a src) and non-obligatory parameters, so perhaps this would be the best:
Html::img( $src, array( 'alt' => $alt ) );
It should not be a problem to remember parameter order when there is only one parameter.
Yet another possibility is to patch PHP so that it allows named parameters :]
Nikola Smolenski wrote:
Ilmari Karonen wrote:
Tim Starling wrote:
Html::img( array( 'src' => $src, 'alt' => $alt ) );
...although at that point you might almost as well just go the extra step and make it:
Html::element( 'img', array( 'src' => $src, 'alt' => $alt ) );
Why not go full way:
Html::doStuff( array( 'what' => 'element', 'element' => 'img', 'src' => $src, 'alt' => $alt ) );
Well, my point was really that the process of constructing a well-formed HTML (or XML) element is pretty much independent of what the element's name is, so any convenience functions like the putative Html::img() just end up being thin wrappers around a generic element-building function anyway. If the wrappers end up *so* thin that they all just look like this:
public static function img( $attribs = array() ) { return self::element( 'img', $attribs ); }
then they don't really add much convenience anymore and might as well be dispensed with.
Seriously, I see a logical distinction between obligatory (every image must have a src) and non-obligatory parameters, so perhaps this would be the best:
Html::img( $src, array( 'alt' => $alt ) );
It should not be a problem to remember parameter order when there is only one parameter.
Nah, surely that would be way too sensible. :)
* Ilmari Karonen nospam@vyznev.net [Tue, 25 Aug 2009 16:54:42 +0300]:
Nikola Smolenski wrote:
Ilmari Karonen wrote:
Tim Starling wrote:
Html::img( array( 'src' => $src, 'alt' => $alt ) );
...although at that point you might almost as well just go the
extra
step and make it:
Html::element( 'img', array( 'src' => $src, 'alt' => $alt ) );
Why not go full way:
Html::doStuff( array( 'what' => 'element', 'element' => 'img', 'src'
=>
$src, 'alt' => $alt ) );
Well, my point was really that the process of constructing a
well-formed
HTML (or XML) element is pretty much independent of what the element's name is, so any convenience functions like the putative Html::img()
just
end up being thin wrappers around a generic element-building function anyway. If the wrappers end up *so* thin that they all just look like this:
public static function img( $attribs = array() ) { return self::element( 'img', $attribs ); }
then they don't really add much convenience anymore and might as well
be
dispensed with.
Seriously, I see a logical distinction between obligatory (every
image
must have a src) and non-obligatory parameters, so perhaps this
would
be
the best:
Html::img( $src, array( 'alt' => $alt ) );
It should not be a problem to remember parameter order when there is only one parameter.
Nah, surely that would be way too sensible. :)
I think that much more important would be building a DOM-like tree from these to be able to add/remove tags and their attributes on the fly. Shouldn't be extremly hard for nested arrays/objects (my extension has it, though in the simplified way). Dmitriy
wikitech-l@lists.wikimedia.org