r74988 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r74987‎ | r74988 | r74989 >
Date:00:44, 19 October 2010
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
Swap usage of create_function for defined function as a callback
Modified paths:
  • /trunk/phase3/includes/api/ApiBase.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiBase.php
——@@ -532,8 +532,8 @@
533533 array_shift( $required );
534534
535535 $intersection = array_intersect( array_keys( array_filter( $params,
536 - create_function( '$x', 'return !is_null($x) && $x !== false;' )
537 - ) ), $required );
 536+ "validateParameter" ) ), $required );
 537+
538538 if ( count( $intersection ) > 1 ) {
539539 $this->dieUsage( 'The parameters ' . implode( ', ', $intersection ) . ' can not be used together', 'invalidparammix' );
540540 } elseif ( count( $intersection ) == 0 ) {
——@@ -541,6 +541,10 @@
542542 }
543543 }
544544
 545+ private function validateParameter( $x ) {
 546+ return !is_null( $x ) && $x !== false;
 547+ }
 548+
545549 /**
546550 * @deprecated use MWNamespace::getValidNamespaces()
547551 */

Follow-up revisions

RevisionCommit summaryAuthorDate
r75025Followup r74988, brion wins again ;Dreedy18:40, 19 October 2010

Comments

#Comment by Brion VIBBER (talk | contribs)   18:10, 19 October 2010

The callback function's name is a bit ambiguous, and could be confused with other functions like validateLimit. Maybe give it a more explicit name like 'parameterNotEmpty', and drop on a documentation comment to indicate what it's used for?

More generally; I'm not sure that call actually works. Shouldn't that be array($this, 'validateParameter')?

Status & tagging log