From a5903d2a53bb3950212b0e87de1a8a17659ac2d7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 9 Apr 2012 11:08:59 -0700 Subject: [PATCH] Use head_key() and last_key() to explicitly communicate intent Summary: PHP arrays have an internal "current position" marker. (I think because foreach() wasn't introduced until PHP 4 and there was no way to get rid of it by then?) A few functions affect the position of the marker, like reset(), end(), each(), next(), and prev(). A few functions read the position of the marker, like each(), next(), prev(), current() and key(). For the most part, no one uses any of this because foreach() is vastly easier and more natural. However, we sometimes want to select the first or last key from an array. Since key() returns the key //at the current position//, and you can't guarantee that no one will introduce some next() calls somewhere, the right way to do this is reset() + key(). This is cumbesome, so we introduced head_key() and last_key() (like head() and last()) in D2161. Switch all the reset()/end() + key() (or omitted reset() since I was feeling like taking risks + key()) calls to head_key() or last_key(). Test Plan: Verified most of these by visiting the affected pages. Reviewers: btrahan, vrana, jungejason, Koolvin Reviewed By: jungejason CC: aran Differential Revision: https://secure.phabricator.com/D2169 --- src/aphront/console/core/DarkConsoleCore.php | 5 ++--- .../console/PhabricatorConduitConsoleController.php | 2 +- .../controller/external/DiffusionExternalController.php | 3 +-- .../herald/controller/home/HeraldHomeController.php | 2 +- .../herald/controller/new/HeraldNewController.php | 3 +-- .../controller/report/ManiphestReportController.php | 6 +++--- .../controller/edit/PhabricatorRepositoryEditController.php | 3 +-- .../svn/PhabricatorRepositorySvnCommitDiscoveryDaemon.php | 4 ++-- .../render/PhabricatorUIExampleRenderController.php | 3 +-- 9 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/aphront/console/core/DarkConsoleCore.php b/src/aphront/console/core/DarkConsoleCore.php index 38a6f0f0fb..1786447786 100644 --- a/src/aphront/console/core/DarkConsoleCore.php +++ b/src/aphront/console/core/DarkConsoleCore.php @@ -1,7 +1,7 @@ getConsoleVisible(); if (!isset($plugins[$selected])) { - reset($plugins); - $selected = key($plugins); + $selected = head_key($plugins); } $tabs = array(); diff --git a/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php b/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php index 010c03ba87..ab945a3a82 100644 --- a/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php +++ b/src/applications/conduit/controller/console/PhabricatorConduitConsoleController.php @@ -34,7 +34,7 @@ final class PhabricatorConduitConsoleController $methods = $this->getAllMethods(); if (empty($methods[$this->method])) { - $this->method = key($methods); + $this->method = head_key($methods); } $this->setFilter('method/'.$this->method); diff --git a/src/applications/diffusion/controller/external/DiffusionExternalController.php b/src/applications/diffusion/controller/external/DiffusionExternalController.php index 4590fb2f13..8ae8dcdba1 100644 --- a/src/applications/diffusion/controller/external/DiffusionExternalController.php +++ b/src/applications/diffusion/controller/external/DiffusionExternalController.php @@ -52,8 +52,7 @@ final class DiffusionExternalController extends DiffusionController { } arsort($matches); - reset($matches); - $best_match = key($matches); + $best_match = head_key($matches); if ($best_match) { $repository = $repositories[$best_match]; diff --git a/src/applications/herald/controller/home/HeraldHomeController.php b/src/applications/herald/controller/home/HeraldHomeController.php index 8d15a70ad4..ba7d877715 100644 --- a/src/applications/herald/controller/home/HeraldHomeController.php +++ b/src/applications/herald/controller/home/HeraldHomeController.php @@ -46,7 +46,7 @@ final class HeraldHomeController extends HeraldController { $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); if (empty($content_type_map[$this->contentType])) { - $this->contentType = key($content_type_map); + $this->contentType = head_key($content_type_map); } $content_desc = $content_type_map[$this->contentType]; diff --git a/src/applications/herald/controller/new/HeraldNewController.php b/src/applications/herald/controller/new/HeraldNewController.php index be74cfdec7..5909604032 100644 --- a/src/applications/herald/controller/new/HeraldNewController.php +++ b/src/applications/herald/controller/new/HeraldNewController.php @@ -33,8 +33,7 @@ final class HeraldNewController extends HeraldController { $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); if (empty($content_type_map[$this->contentType])) { - reset($content_type_map); - $this->contentType = key($content_type_map); + $this->contentType = head_key($content_type_map); } $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); diff --git a/src/applications/maniphest/controller/report/ManiphestReportController.php b/src/applications/maniphest/controller/report/ManiphestReportController.php index 91a95a96e4..fc8818124b 100644 --- a/src/applications/maniphest/controller/report/ManiphestReportController.php +++ b/src/applications/maniphest/controller/report/ManiphestReportController.php @@ -173,7 +173,7 @@ final class ManiphestReportController extends ManiphestController { $week = null; $month = null; - $last = key($stats) - 1; + $last = last_key($stats) - 1; $period = $template; foreach ($stats as $bucket => $info) { @@ -182,7 +182,7 @@ final class ManiphestReportController extends ManiphestController { $week_bucket = phabricator_format_local_time( $epoch, $user, - 'W'); + 'YW'); if ($week_bucket != $last_week) { if ($week) { $rows[] = $this->formatBurnRow( @@ -198,7 +198,7 @@ final class ManiphestReportController extends ManiphestController { $month_bucket = phabricator_format_local_time( $epoch, $user, - 'm'); + 'Ym'); if ($month_bucket != $last_month) { if ($month) { $rows[] = $this->formatBurnRow( diff --git a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php index 1e5e0f790b..da29909d43 100644 --- a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php +++ b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php @@ -46,8 +46,7 @@ final class PhabricatorRepositoryEditController $this->repository = $repository; if (!isset($views[$this->view])) { - reset($views); - $this->view = key($views); + $this->view = head_key($views); } $nav = new AphrontSideNavView(); diff --git a/src/applications/repository/daemon/commitdiscovery/svn/PhabricatorRepositorySvnCommitDiscoveryDaemon.php b/src/applications/repository/daemon/commitdiscovery/svn/PhabricatorRepositorySvnCommitDiscoveryDaemon.php index 360e80713e..c3a7a2fbfe 100644 --- a/src/applications/repository/daemon/commitdiscovery/svn/PhabricatorRepositorySvnCommitDiscoveryDaemon.php +++ b/src/applications/repository/daemon/commitdiscovery/svn/PhabricatorRepositorySvnCommitDiscoveryDaemon.php @@ -33,8 +33,8 @@ final class PhabricatorRepositorySvnCommitDiscoveryDaemon $uri); $results = $this->parseSVNLogXML($xml); - $commit = key($results); - $epoch = reset($results); + $commit = head_key($results); + $epoch = head($results); if ($this->isKnownCommit($commit)) { return false; diff --git a/src/applications/uiexample/controller/render/PhabricatorUIExampleRenderController.php b/src/applications/uiexample/controller/render/PhabricatorUIExampleRenderController.php index 25294c5e1b..3cd4165565 100644 --- a/src/applications/uiexample/controller/render/PhabricatorUIExampleRenderController.php +++ b/src/applications/uiexample/controller/render/PhabricatorUIExampleRenderController.php @@ -41,8 +41,7 @@ final class PhabricatorUIExampleRenderController } if (!$selected) { - reset($classes); - $selected = key($classes); + $selected = head_key($classes); } $nav = new AphrontSideNavView();