r87381 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r87380‎ | r87381 | r87382 >
Date:01:51, 4 May 2011
Author:awjrichards
Status:resolved (Comments)
Tags:
Comment:
Now actually determing only the top and bottom 50 rated pages
Modified paths:
  • /trunk/extensions/ArticleFeedback/populateAFStatistics.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedback/populateAFStatistics.php
@@ -17,7 +17,7 @@
1818 * The period (in seconds) before now for which to gather stats
1919 * @var int
2020 */
21 - public $polling_period = 86400;
 21+ public $polling_period = 1000000;//86400;
2222
2323 /**
2424 * The formatted timestamp from which to determine stats
@@ -62,6 +62,7 @@
6363 $ratings = array();
6464
6565 // fetch the ratings since the lower bound timestamp
 66+ $this->output( 'Fetching page ratings between now and ' . date('Y-m-d H:i:s', strtotime( $this->getLowerBoundTimestamp())) . "...\n");
6667 $res = $this->dbr->select(
6768 'article_feedback',
6869 array(
@@ -73,14 +74,17 @@
7475 __METHOD__,
7576 array() // better to do with limits and offsets?
7677 );
77 -
 78+
7879 // assign the rating data to our data structure
7980 foreach ( $res as $row ) {
8081 $ratings[ $row->aa_page_id ][ $row->aa_rating_id ][] = $row->aa_rating_value;
8182 }
 83+ $this->output( "Done\n" );
8284
8385 // determine the average ratings for a given page
 86+ $this->output( "Determining average ratings for articles ...\n" );
8487 $highs_and_lows = array();
 88+ $averages = array();
8589 foreach ( $ratings as $page_id => $data ) {
8690 foreach( $data as $rating_id => $rating ) {
8791 $rating_sum = array_sum( $rating );
@@ -90,13 +94,31 @@
9195 $overall_rating_sum = array_sum( $highs_and_lows[ $page_id ][ 'avg_ratings' ] );
9296 $overall_rating_average = $overall_rating_sum / count( $highs_and_lows[ $page_id ][ 'avg_ratings' ] );
9397 $highs_and_lows[ $page_id ][ 'average' ] = $overall_rating_average;
 98+
 99+ // store overall average rating seperately so we can easily sort
 100+ $averages[ $page_id ] = $overall_rating_average;
94101 }
 102+ $this->output( "Done.\n" );
 103+
 104+ // determine highest 50 and lowest 50
 105+ $this->output( "Determining 50 highest and 50 lowest rated articles...\n" );
 106+ asort( $averages );
 107+ // take lowest 50 and highest 50
 108+ $highest_and_lowest_page_ids = array_slice( $averages, 0, 50, true );
 109+ if ( count( $averages ) > 50 ) {
 110+ $highest_and_lowest_page_ids = array_merge( $highest_and_lowest_page_ids, array_slice( $averages, -50, 50, true ));
 111+ }
 112+ $this->output( "Done\n" );
95113
96114 // prepare data for insert into db
97115 $this->output( "Preparing data for db insertion ...\n");
98116 $cur_ts = $this->dbw->timestamp();
99117 $rows = array();
100118 foreach( $highs_and_lows as $page_id => $data ) {
 119+ // make sure this is one of the highest/lowest average ratings
 120+ if ( !in_array( $page_id, array_keys( $highest_and_lowest_page_ids ))) {
 121+ continue;
 122+ }
101123 $rows[] = array(
102124 'afshl_page_id' => $page_id,
103125 'afshl_avg_overall' => $data[ 'average' ],
@@ -105,7 +127,7 @@
106128 );
107129 }
108130 $this->output( "Done.\n" );
109 -
 131+
110132 // insert data to db
111133 $this->output( "Writing data to article_feedback_stats_highs_lows ...\n" );
112134 $rowsInserted = 0;

Follow-up revisions

RevisionCommit summaryAuthorDate
r87384Unpatched local version of ArticleFeedback to replace original require_once p...awjrichards02:10, 4 May 2011
r87522Followup r87402, r87381, r87379...awjrichards00:14, 6 May 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:47, 4 May 2011

Intented:

public $polling_period = 1000000;//86400;

?

#Comment by Nikerabbit (talk | contribs)   07:51, 4 May 2011

Nevermind. No follow-up was registered so I didn't know I would be writing useless comments.

#Comment by Catrope (talk | contribs)   19:23, 4 May 2011
+		if ( count( $averages ) > 50 ) {
+			$highest_and_lowest_page_ids = array_merge( $highest_and_lowest_page_ids, array_slice( $averages, -50, 50, true ));
+		}

Don't use array_merge() , use the plus operator in this case. If there are between 51 and 99 elements in $averages, the left and right side will overlap, and array_merge() will shift the (integral!) keys to avoid key collisions, while the plus operator just lets the left(?) side win.

+			if ( !in_array( $page_id, array_keys( $highest_and_lowest_page_ids ))) {

That's a really elaborate way of saying !isset( $highest_and_lowest_page_ids[$page_id] ) ;)

#Comment by Awjrichards (talk | contribs)   19:41, 4 May 2011

> Don't use array_merge() , use the plus operator in this case. If there are between 51 and 99 elements in $averages, the left and right side will overlap, and array_merge() will shift the (integral!) keys to avoid key collisions, while the plus operator just lets the left(?) side win.

Oh you are absolutely right, array_merge only overwrites when keys are strings - good catch

> That's a really elaborate way of saying !isset( $highest_and_lowest_page_ids[$page_id] ) ;)

Ahh.. yeah :-/ Heh. Fixes coming up.

Status & tagging log