r113518 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r113517‎ | r113518 | r113519 >
Date:20:26, 9 March 2012
Author:ialex
Status:ok (Comments)
Tags:
Comment:
No need to create a new Revision object if we already have one available
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Article.php
——@@ -1093,7 +1093,12 @@
10941094 $extraParams['unhide'] = 1;
10951095 }
10961096
1097 - $revision = Revision::newFromId( $oldid );
 1097+ if ( $this->mRevision && $this->mRevision->getId() === $oldid ) {
 1098+ $revision = $this->mRevision;
 1099+ } else {
 1100+ $revision = Revision::newFromId( $oldid );
 1101+ }
 1102+
10981103 $timestamp = $revision->getTimestamp();
10991104
11001105 $current = ( $oldid == $this->mPage->getLatest() );

Comments

#Comment by Krinkle (talk | contribs)   13:40, 10 March 2012

Would it make sense to apply a similar change to the Revision instantiation from oldid around line 254 in Article::getOldIDFromRequest ?

#Comment by IAlex (talk | contribs)   17:13, 10 March 2012

Isn't that r113468?

#Comment by Krinkle (talk | contribs)   03:11, 11 March 2012

No, I was looking at the code as of this rev (that rev was before this one). 

			if ( $oldid === $this->mPage->getLatest() ) {
 				$this->mRevision = $this->mPage->getRevision();
 			} else {
 				$this->mRevision = Revision::newFromId( $oldid );

I don't know the context here, but if it's possible that mRevision is already defined, it could use an additional if ( $this->mRevision && $this->mRevision->getId() !== $oldid ) { $this->mRevision = Revision::newFromId( $oldid ); } wrap.

If not, then it's perfectly fine.

#Comment by IAlex (talk | contribs)   19:08, 13 March 2012

Article::$mRevision is only set in Article::getOldIDFromRequest() and Article::fetchContent(), but the latter will call the former before executing the relevant part of code, so Article::getOldIDFromRequest() cannot be called with a Revision object being set.

#Comment by Krinkle (talk | contribs)   20:21, 13 March 2012

Nice :)

Status & tagging log