Fix documentation of HistoryPager::lastLink() and make code more readable

- Describe the possible types of $next parameter in the documentation
  rather than inline along with the possible values
- Split the big if ... elseif block into separate blocks. The elseif
  are not needed since each path returns
- Only create the revision object when really necessary

Change-Id: Ic92a6f6bd405d3f820d562a7322d34e3d9d33d17

diff --git a/includes/actions/HistoryAction.php b/includes/actions/HistoryAction.php
index c946184..b0274ff 100644 (file)
--- a/includes/actions/HistoryAction.php
+++ b/includes/actions/HistoryAction.php
@@ -776,18 +776,21 @@ class HistoryPager extends ReverseChronologicalPager {
        /**
         * Create a diff-to-previous link for this revision for this page.
         *
-        * @param Revision $prevRev The previous revision
-        * @param mixed $next The newer revision
+        * @param Revision $prevRev The revision being displayed
+        * @param stdClass|string|null $next The next revision in list (that is
+        *        the previous one in chronological order).
+        *        May either be a row, "unknown" or null.
         * @return string
         */
        function lastLink( $prevRev, $next ) {
                $last = $this->historyPage->message['last'];
-               # $next may either be a Row, null, or "unknown"
-               $nextRev = is_object( $next ) ? new Revision( $next ) : $next;
-               if ( is_null( $next ) ) {
+
+               if ( $next === null ) {
                        # Probably no next row
                        return $last;
-               } elseif ( $next === 'unknown' ) {
+               }
+
+               if ( $next === 'unknown' ) {
                        # Next row probably exists but is unknown, use an oldid=prev link
                        return Linker::linkKnown(
                                $this->getTitle(),
@@ -798,21 +801,25 @@ class HistoryPager extends ReverseChronologicalPager {
                                        'oldid' => 'prev'
                                )
                        );
-               } elseif ( !$prevRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
+               }
+
+               $nextRev = new Revision( $next );
+
+               if ( !$prevRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
                        || !$nextRev->userCan( Revision::DELETED_TEXT, $this->getUser() )
                ) {
                        return $last;
-               } else {
-                       return Linker::linkKnown(
-                               $this->getTitle(),
-                               $last,
-                               array(),
-                               array(
-                                       'diff' => $prevRev->getId(),
-                                       'oldid' => $next->rev_id
-                               )
-                       );
                }
+
+               return Linker::linkKnown(
+                       $this->getTitle(),
+                       $last,
+                       array(),
+                       array(
+                               'diff' => $prevRev->getId(),
+                               'oldid' => $next->rev_id
+                       )
+               );
        }
 
        /**