r89273 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89272‎ | r89273 | r89274 >
Date:16:40, 1 June 2011
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Add some documentation

Swap if ( $foo ) { $this->addFields( 'foo' ); } for $this->addFieldsIf( 'foo', $foo ); etc

Also swap $this->addFieldsIf( 'foo', $foo ); $this->addFieldsIf( 'foo2', $foo ); for $this->addFieldsIf( array( 'foo', 'foo2' ), $foo );


Micro-optimisation and readability!
Modified paths:
  • /trunk/phase3/includes/api/ApiQueryBase.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryBlocks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryDeletedrevs.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryFilearchive.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLogEvents.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryRecentChanges.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryTags.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryUserContributions.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryWatchlist.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiQueryRecentChanges.php
——@@ -224,22 +224,14 @@
225225 }
226226
227227 /* Add fields to our query if they are specified as a needed parameter. */
228 - $this->addFieldsIf( 'rc_id', $this->fld_ids );
229 - $this->addFieldsIf( 'rc_this_oldid', $this->fld_ids );
230 - $this->addFieldsIf( 'rc_last_oldid', $this->fld_ids );
 228+ $this->addFieldsIf( array( 'rc_id', 'rc_this_oldid', 'rc_last_oldid' ), $this->fld_ids );
231229 $this->addFieldsIf( 'rc_comment', $this->fld_comment || $this->fld_parsedcomment );
232230 $this->addFieldsIf( 'rc_user', $this->fld_user );
233231 $this->addFieldsIf( 'rc_user_text', $this->fld_user || $this->fld_userid );
234 - $this->addFieldsIf( 'rc_minor', $this->fld_flags );
235 - $this->addFieldsIf( 'rc_bot', $this->fld_flags );
236 - $this->addFieldsIf( 'rc_new', $this->fld_flags );
237 - $this->addFieldsIf( 'rc_old_len', $this->fld_sizes );
238 - $this->addFieldsIf( 'rc_new_len', $this->fld_sizes );
 232+ $this->addFieldsIf( array( 'rc_minor', 'rc_new', 'rc_bot' ) , $this->fld_flags );
 233+ $this->addFieldsIf( array( 'rc_old_len', 'rc_new_len' ), $this->fld_sizes );
239234 $this->addFieldsIf( 'rc_patrolled', $this->fld_patrolled );
240 - $this->addFieldsIf( 'rc_logid', $this->fld_loginfo );
241 - $this->addFieldsIf( 'rc_log_type', $this->fld_loginfo );
242 - $this->addFieldsIf( 'rc_log_action', $this->fld_loginfo );
243 - $this->addFieldsIf( 'rc_params', $this->fld_loginfo );
 235+ $this->addFieldsIf( array( 'rc_logid', 'rc_log_type', 'rc_log_action', 'rc_params' ), $this->fld_loginfo );
244236 $showRedirects = $this->fld_redirect || isset( $show['redirect'] ) || isset( $show['!redirect'] );
245237 }
246238
Index: trunk/phase3/includes/api/ApiQueryDeletedrevs.php
——@@ -99,27 +99,14 @@
100100 $this->addWhere( 'ar_deleted = 0' );
101101 $this->addFields( array( 'ar_title', 'ar_namespace', 'ar_timestamp' ) );
102102
103 - if ( $fld_parentid ) {
104 - $this->addFields( 'ar_parent_id' );
105 - }
106 - if ( $fld_revid ) {
107 - $this->addFields( 'ar_rev_id' );
108 - }
109 - if ( $fld_user ) {
110 - $this->addFields( 'ar_user_text' );
111 - }
112 - if ( $fld_userid ) {
113 - $this->addFields( 'ar_user' );
114 - }
115 - if ( $fld_comment || $fld_parsedcomment ) {
116 - $this->addFields( 'ar_comment' );
117 - }
118 - if ( $fld_minor ) {
119 - $this->addFields( 'ar_minor_edit' );
120 - }
121 - if ( $fld_len ) {
122 - $this->addFields( 'ar_len' );
123 - }
 103+ $this->addFieldsIf( 'ar_parent_id', $fld_parentid );
 104+ $this->addFieldsIf( 'ar_rev_id', $fld_revid );
 105+ $this->addFieldsIf( 'ar_user_text', $fld_user );
 106+ $this->addFieldsIf( 'ar_user', $fld_userid );
 107+ $this->addFieldsIf( 'ar_comment', $fld_comment || $fld_parsedcomment );
 108+ $this->addFieldsIf( 'ar_minor_edit', $fld_minor );
 109+ $this->addFieldsIf( 'ar_len', $fld_len );
 110+
124111 if ( $fld_content ) {
125112 $this->addTables( 'text' );
126113 $this->addFields( array( 'ar_text', 'ar_text_id', 'old_text', 'old_flags' ) );
Index: trunk/phase3/includes/api/ApiQueryBase.php
——@@ -111,7 +111,7 @@
112112
113113 /**
114114 * Add a set of fields to select to the internal array
115 - * @param $value mixed Field name or array of field names
 115+ * @param $value array|string Field name or array of field names
116116 */
117117 protected function addFields( $value ) {
118118 if ( is_array( $value ) ) {
——@@ -123,7 +123,7 @@
124124
125125 /**
126126 * Same as addFields(), but add the fields only if a condition is met
127 - * @param $value mixed See addFields()
 127+ * @param $value array|string See addFields()
128128 * @param $condition bool If false, do nothing
129129 * @return bool $condition
130130 */
Index: trunk/phase3/includes/api/ApiQueryBlocks.php
——@@ -70,33 +70,17 @@
7171 $this->addTables( 'ipblocks' );
7272 $this->addFields( 'ipb_auto' );
7373
74 - if ( $fld_id ) {
75 - $this->addFields( 'ipb_id' );
76 - }
77 - if ( $fld_user || $fld_userid ) {
78 - $this->addFields( array( 'ipb_address', 'ipb_user' ) );
79 - }
80 - if ( $fld_by ) {
81 - $this->addFields( 'ipb_by_text' );
82 - }
83 - if ( $fld_byid ) {
84 - $this->addFields( 'ipb_by' );
85 - }
86 - if ( $fld_timestamp ) {
87 - $this->addFields( 'ipb_timestamp' );
88 - }
89 - if ( $fld_expiry ) {
90 - $this->addFields( 'ipb_expiry' );
91 - }
92 - if ( $fld_reason ) {
93 - $this->addFields( 'ipb_reason' );
94 - }
95 - if ( $fld_range ) {
96 - $this->addFields( array( 'ipb_range_start', 'ipb_range_end' ) );
97 - }
98 - if ( $fld_flags ) {
99 - $this->addFields( array( 'ipb_anon_only', 'ipb_create_account', 'ipb_enable_autoblock', 'ipb_block_email', 'ipb_deleted', 'ipb_allow_usertalk' ) );
100 - }
 74+ $this->addFieldsIf ( 'ipb_id', $fld_id );
 75+ $this->addFieldsIf( array( 'ipb_address', 'ipb_user' ), $fld_user || $fld_userid );
 76+ $this->addFieldsIf( 'ipb_by_text', $fld_by );
 77+ $this->addFieldsIf( 'ipb_by', $fld_byid );
 78+ $this->addFieldsIf( 'ipb_timestamp', $fld_timestamp );
 79+ $this->addFieldsIf( 'ipb_expiry', $fld_expiry );
 80+ $this->addFieldsIf( 'ipb_reason', $fld_reason );
 81+ $this->addFieldsIf( array( 'ipb_range_start', 'ipb_range_end' ), $fld_range );
 82+ $this->addFieldsIf( array( 'ipb_anon_only', 'ipb_create_account', 'ipb_enable_autoblock',
 83+ 'ipb_block_email', 'ipb_deleted', 'ipb_allow_usertalk' ),
 84+ $fld_flags );
10185
10286 $this->addOption( 'LIMIT', $params['limit'] + 1 );
10387 $this->addWhereRange( 'ipb_timestamp', $params['dir'], $params['start'], $params['end'] );
Index: trunk/phase3/includes/api/ApiQueryFilearchive.php
——@@ -69,21 +69,10 @@
7070 $this->addFields( array( 'fa_name', 'fa_deleted' ) );
7171 $this->addFieldsIf( 'fa_storage_key', $fld_sha1 );
7272 $this->addFieldsIf( 'fa_timestamp', $fld_timestamp );
73 -
74 - if ( $fld_user ) {
75 - $this->addFields( array( 'fa_user', 'fa_user_text' ) );
76 - }
77 -
78 - if ( $fld_dimensions || $fld_size ) {
79 - $this->addFields( array( 'fa_height', 'fa_width', 'fa_size' ) );
80 - }
81 -
 73+ $this->addFieldsIf( array( 'fa_user', 'fa_user_text' ), $fld_user );
 74+ $this->addFieldsIf( array( 'fa_height', 'fa_width', 'fa_size' ), $fld_dimensions || $fld_size );
8275 $this->addFieldsIf( 'fa_description', $fld_description );
83 -
84 - if ( $fld_mime ) {
85 - $this->addFields( array( 'fa_major_mime', 'fa_minor_mime' ) );
86 - }
87 -
 76+ $this->addFieldsIf( array( 'fa_major_mime', 'fa_minor_mime' ), $fld_mime );
8877 $this->addFieldsIf( 'fa_metadata', $fld_metadata );
8978 $this->addFieldsIf( 'fa_bits', $fld_bitdepth );
9079
Index: trunk/phase3/includes/api/ApiQueryLogEvents.php
——@@ -86,13 +86,10 @@
8787 'log_deleted',
8888 ) );
8989
90 - $this->addFieldsIf( 'log_id', $this->fld_ids );
91 - $this->addFieldsIf( 'page_id', $this->fld_ids );
92 - $this->addFieldsIf( 'log_user', $this->fld_user );
93 - $this->addFieldsIf( 'user_name', $this->fld_user );
 90+ $this->addFieldsIf( array( 'log_id', 'page_id' ), $this->fld_ids );
 91+ $this->addFieldsIf( array( 'log_user', 'user_name' ), $this->fld_user );
9492 $this->addFieldsIf( 'user_id', $this->fld_userid );
95 - $this->addFieldsIf( 'log_namespace', $this->fld_title || $this->fld_parsedcomment );
96 - $this->addFieldsIf( 'log_title', $this->fld_title || $this->fld_parsedcomment );
 93+ $this->addFieldsIf( array( 'log_namespace', 'log_title' ), $this->fld_title || $this->fld_parsedcomment );
9794 $this->addFieldsIf( 'log_comment', $this->fld_comment || $this->fld_parsedcomment );
9895 $this->addFieldsIf( 'log_params', $this->fld_details );
9996
Index: trunk/phase3/includes/api/ApiQueryWatchlist.php
——@@ -101,20 +101,14 @@
102102 'rc_last_oldid',
103103 ) );
104104
105 - $this->addFieldsIf( 'rc_new', $this->fld_flags );
106 - $this->addFieldsIf( 'rc_minor', $this->fld_flags );
107 - $this->addFieldsIf( 'rc_bot', $this->fld_flags );
 105+ $this->addFieldsIf( array( 'rc_new', 'rc_minor', 'rc_bot' ), $this->fld_flags );
108106 $this->addFieldsIf( 'rc_user', $this->fld_user || $this->fld_userid );
109107 $this->addFieldsIf( 'rc_user_text', $this->fld_user );
110108 $this->addFieldsIf( 'rc_comment', $this->fld_comment || $this->fld_parsedcomment );
111109 $this->addFieldsIf( 'rc_patrolled', $this->fld_patrol );
112 - $this->addFieldsIf( 'rc_old_len', $this->fld_sizes );
113 - $this->addFieldsIf( 'rc_new_len', $this->fld_sizes );
 110+ $this->addFieldsIf( array( 'rc_old_len', 'rc_new_len' ), $this->fld_sizes );
114111 $this->addFieldsIf( 'wl_notificationtimestamp', $this->fld_notificationtimestamp );
115 - $this->addFieldsIf( 'rc_logid', $this->fld_loginfo );
116 - $this->addFieldsIf( 'rc_log_type', $this->fld_loginfo );
117 - $this->addFieldsIf( 'rc_log_action', $this->fld_loginfo );
118 - $this->addFieldsIf( 'rc_params', $this->fld_loginfo );
 112+ $this->addFieldsIf( array( 'rc_logid', 'rc_log_type', 'rc_log_action', 'rc_params' ), $this->fld_loginfo );
119113 } elseif ( $params['allrev'] ) {
120114 $this->addFields( 'rc_this_oldid' );
121115 } else {
Index: trunk/phase3/includes/api/ApiQueryTags.php
——@@ -64,9 +64,7 @@
6565 $this->addTables( 'change_tag' );
6666 $this->addFields( 'ct_tag' );
6767
68 - if ( $this->fld_hitcount ) {
69 - $this->addFields( 'count(*) AS hitcount' );
70 - }
 68+ $this->addFieldsIf( 'count(*) AS hitcount', $this->fld_hitcount );
7169
7270 $this->addOption( 'LIMIT', $this->limit + 1 );
7371 $this->addOption( 'GROUP BY', 'ct_tag' );
Index: trunk/phase3/includes/api/ApiQueryUserContributions.php
——@@ -249,8 +249,7 @@
250250 // $this->addFieldsIf( 'rev_text_id', $this->fld_ids ); // Should this field be exposed?
251251 $this->addFieldsIf( 'rev_comment', $this->fld_comment || $this->fld_parsedcomment );
252252 $this->addFieldsIf( 'rev_len', $this->fld_size );
253 - $this->addFieldsIf( 'rev_minor_edit', $this->fld_flags );
254 - $this->addFieldsIf( 'rev_parent_id', $this->fld_flags );
 253+ $this->addFieldsIf( array( 'rev_minor_edit', 'rev_parent_id' ), $this->fld_flags );
255254 $this->addFieldsIf( 'rc_patrolled', $this->fld_patrolled );
256255
257256 if ( $this->fld_tags ) {

Comments

#Comment by Brion VIBBER (talk | contribs)   21:00, 3 June 2011

Looks ok and doesn't seem to cause any new phpunit failures in API tests.

Status & tagging log