r102177 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r102176‎ | r102177 | r102178 >
Date:15:02, 6 November 2011
Author:ashley
Status:deferred (Comments)
Tags:socialprofile 
Comment:
SocialProfile: as per Markus' in-depth review:
*var -> private
*prevent empty IN() clauses in SQL by checking that the variable is *not* empty before using it in the SQL query
*move global variable declaration out of the foreach loop in setSystemGiftsRec()
Modified paths:
  • /trunk/extensions/SocialProfile/UserActivity/UserActivityClass.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SocialProfile/UserActivity/UserActivityClass.php
——@@ -9,22 +9,19 @@
1010 * Please use the accessor functions
1111 */
1212
13 - /**#@+
14 - * @private
15 - */
16 - var $user_id; # Text form (spaces not underscores) of the main part
17 - var $user_name; # Text form (spaces not underscores) of the main part
18 - var $items; # Text form (spaces not underscores) of the main part
19 - var $rel_type;
20 - var $show_edits = 1;
21 - var $show_votes = 0;
22 - var $show_comments = 1;
23 - var $show_relationships = 1;
24 - var $show_gifts_sent = 0;
25 - var $show_gifts_rec = 1;
26 - var $show_system_gifts = 1;
27 - var $show_system_messages = 1;
28 - var $show_messages_sent = 1;
 13+ private $user_id; # Text form (spaces not underscores) of the main part
 14+ private $user_name; # Text form (spaces not underscores) of the main part
 15+ private $items; # Text form (spaces not underscores) of the main part
 16+ private $rel_type;
 17+ private $show_edits = 1;
 18+ private $show_votes = 0;
 19+ private $show_comments = 1;
 20+ private $show_relationships = 1;
 21+ private $show_gifts_sent = 0;
 22+ private $show_gifts_rec = 1;
 23+ private $show_system_gifts = 1;
 24+ private $show_system_messages = 1;
 25+ private $show_messages_sent = 1;
2926
3027 /**
3128 * Constructor
——@@ -94,7 +91,9 @@
9592 $userArray[] = $user;
9693 }
9794 $userIDs = implode( ',', $userArray );
98 - $where[] = "rc_user IN ($userIDs)";
 95+ if ( !empty( $userIDs ) ) {
 96+ $where[] = "rc_user IN ($userIDs)";
 97+ }
9998 }
10099
101100 if ( !empty( $this->show_current_user ) ) {
——@@ -187,7 +186,9 @@
188187 $userArray[] = $user;
189188 }
190189 $userIDs = implode( ',', $userArray );
191 - $where[] = "vote_user_id IN ($userIDs)";
 190+ if ( !empty( $userIDs ) ) {
 191+ $where[] = "vote_user_id IN ($userIDs)";
 192+ }
192193 }
193194 if ( $this->show_current_user ) {
194195 $where['vote_user_id'] = $this->user_id;
——@@ -256,7 +257,9 @@
257258 $userArray[] = $user;
258259 }
259260 $userIDs = implode( ',', $userArray );
260 - $where[] = "Comment_user_id IN ($userIDs)";
 261+ if ( !empty( $userIDs ) ) {
 262+ $where[] = "Comment_user_id IN ($userIDs)";
 263+ }
261264 }
262265
263266 if ( !empty( $this->show_current_user ) ) {
——@@ -348,7 +351,9 @@
349352 $userArray[] = $user;
350353 }
351354 $userIDs = implode( ',', $userArray );
352 - $where[] = "ug_user_id_to IN ($userIDs)";
 355+ if ( !empty( $userIDs ) ) {
 356+ $where[] = "ug_user_id_to IN ($userIDs)";
 357+ }
353358 }
354359
355360 if( $this->show_current_user ) {
——@@ -412,7 +417,9 @@
413418 $userArray[] = $user;
414419 }
415420 $userIDs = implode( ',', $userArray );
416 - $where[] = "ug_user_id_to IN ($userIDs)";
 421+ if ( !empty( $userIDs ) ) {
 422+ $where[] = "ug_user_id_to IN ($userIDs)";
 423+ }
417424 }
418425
419426 if ( !empty( $this->show_current_user ) ) {
——@@ -484,6 +491,8 @@
485492 * variables.
486493 */
487494 private function setSystemGiftsRec() {
 495+ global $wgUploadPath;
 496+
488497 $dbr = wfGetDB( DB_SLAVE );
489498
490499 $where = array();
——@@ -503,7 +512,9 @@
504513 $userArray[] = $user;
505514 }
506515 $userIDs = implode( ',', $userArray );
507 - $where[] = "sg_user_id IN ($userIDs)";
 516+ if ( !empty( $userIDs ) ) {
 517+ $where[] = "sg_user_id IN ($userIDs)";
 518+ }
508519 }
509520
510521 if ( !empty( $this->show_current_user ) ) {
——@@ -527,7 +538,6 @@
528539 );
529540
530541 foreach ( $res as $row ) {
531 - global $wgUploadPath;
532542 $user_title = Title::makeTitle( NS_USER, $row->sg_user_name );
533543 $system_gift_image = '<img src="' . $wgUploadPath . '/awards/' .
534544 SystemGifts::getGiftImage( $row->gift_id, 'm' ) .
——@@ -589,7 +599,9 @@
590600 $userArray[] = $user;
591601 }
592602 $userIDs = implode( ',', $userArray );
593 - $where[] = "r_user_id IN ($userIDs)";
 603+ if ( !empty( $userIDs ) ) {
 604+ $where[] = "r_user_id IN ($userIDs)";
 605+ }
594606 }
595607
596608 if ( !empty( $this->show_current_user ) ) {
——@@ -678,7 +690,9 @@
679691 $userArray[] = $user;
680692 }
681693 $userIDs = implode( ',', $userArray );
682 - $where[] = "ub_user_id_from IN ($userIDs)";
 694+ if ( !empty( $userIDs ) ) {
 695+ $where[] = "ub_user_id_from IN ($userIDs)";
 696+ }
683697 }
684698
685699 if ( !empty( $this->show_current_user ) ) {
——@@ -768,7 +782,9 @@
769783 $userArray[] = $user;
770784 }
771785 $userIDs = implode( ',', $userArray );
772 - $where[] = "um_user_id IN ($userIDs)";
 786+ if ( !empty( $userIDs ) ) {
 787+ $where[] = "um_user_id IN ($userIDs)";
 788+ }
773789 }
774790
775791 if ( !empty( $this->show_current_user ) ) {

Comments

#Comment by Nikerabbit (talk | contribs)   15:43, 6 November 2011

If you only want to check if array is empty, use count() instead.

Status & tagging log