r72181 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r72180‎ | r72181 | r72182 >
Date:23:58, 1 September 2010
Author:reedy
Status:reverted (Comments)
Tags:
Comment:
Add a "isInCategory" function to check if a specific title is in a certain category
Modified paths:
  • /trunk/phase3/includes/Title.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Title.php
——@@ -3530,6 +3530,31 @@
35313531 return !$this->isExternal() && MWNamespace::isWatchable( $this->getNamespace() );
35323532 }
35333533
 3534+ /*
 3535+ * Returns whether an article is in the specific category
 3536+ *
 3537+ * @param $category String: The category name (without Category: Prefix)
 3538+ *
 3539+ * @return bool
 3540+ */
 3541+ public function isInCategory( $category ) {
 3542+ $dbr = wfGetDB( DB_SLAVE );
 3543+ $res = $dbr->select(
 3544+ 'categorylinks',
 3545+ '*',
 3546+ array(
 3547+ 'cl_from' => $this->getArticleId(),
 3548+ 'cl_to' => $category,
 3549+ ),
 3550+ __METHOD__,
 3551+ array(
 3552+ 'LIMIT' => 1
 3553+ )
 3554+ );
 3555+
 3556+ return ( $dbr->numRows( $res ) > 0 );
 3557+ }
 3558+
35343559 /**
35353560 * Get categories to which this Title belongs and return an array of
35363561 * categories' names.
——@@ -3553,9 +3578,10 @@
35543579 $res = $dbr->query( $sql );
35553580
35563581 if ( $dbr->numRows( $res ) > 0 ) {
3557 - foreach ( $res as $row )
 3582+ foreach ( $res as $row ) {
35583583 // $data[] = Title::newFromText($wgContLang->getNSText ( NS_CATEGORY ).':'.$row->cl_to);
35593584 $data[$wgContLang->getNSText( NS_CATEGORY ) . ':' . $row->cl_to] = $this->getFullText();
 3585+ }
35603586 } else {
35613587 $data = array();
35623588 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r72200Followup r72181, condense code, use selectRow()reedy16:03, 2 September 2010
r72227Per r72181, refactor isInCategory into ArticleAssessmentPilotreedy20:52, 2 September 2010

Comments

#Comment by Nikerabbit (talk | contribs)   07:01, 2 September 2010

Would selectRow be suitable? And is there any possibility to use less than 12 lines for simple query?

#Comment by Simetrical (talk | contribs)   20:45, 2 September 2010

Is it really necessary to add another Title method with only one caller? This implementation is inefficient for most use-cases – it only makes sense if you're really only doing one test per page view, otherwise you need to batch them. I think it would make more sense to just do the query directly in ArticleAssessmentPilot, since I don't foresee many other callers coming up where this one-by-one query would be useful. Title is huge enough as it stands.

Also, you should select '1' instead of '*'. This will tell you if the row exists, which is all you care about. It allows the DBMS to optimize by not having to hit the table contents, at least for InnoDB. This isn't really important for this query, if it will only be called a couple times per view, but it's a good idea to follow good practice even where it doesn't make a difference so that you (and people who read your code) will remember to follow it where it does make a difference.

Status & tagging log