C++ readability review from original change https://chromiumcodereview.appspot.com/14263024/

chrome/test/chromedriver/logging.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/test/chromedriver/logging.h"
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/time.h"
 #include "chrome/test/chromedriver/capabilities.h"
 #include "chrome/test/chromedriver/chrome/console_logger.h"
 #include "chrome/test/chromedriver/chrome/performance_logger.h"
 #include "chrome/test/chromedriver/chrome/status.h"
 
 
 namespace {
 
 // Map between WebDriverLog::WebDriverLevel and its name in WD wire protocol.
 // Array indices are the WebDriverLog::WebDriverLevel enum values.
-const char* kWebDriverLevelNames[] = {
+const char* const kWebDriverLevelNames[] = {
   "ALL", "DEBUG", "INFO", "WARNING", "SEVERE", "OFF"
 };
 
 // Map between Log::Level and WebDriverLog::WebDriverLevel.
 // Array indices are the Log::Level enum values.
 WebDriverLog::WebDriverLevel kLogLevelToWebDriverLevels[] = {
   WebDriverLog::kWdDebug,  // kDebug
   WebDriverLog::kWdInfo,  // kLog
   WebDriverLog::kWdWarning,  // kWarning
   WebDriverLog::kWdSevere  // kError
 };
 
+// Translates Log::Level to WebDriverLog::WebDriverLevel.
 WebDriverLog::WebDriverLevel LogLevelToWebDriverLevel(Log::Level level) {
   const int index = level - Log::kDebug;
-  CHECK(index >= 0);
-  CHECK(static_cast<size_t>(index) < arraysize(kLogLevelToWebDriverLevels));
+  CHECK_GE(index, 0);
+  CHECK_LT(static_cast<size_t>(index), arraysize(kLogLevelToWebDriverLevels));
   return kLogLevelToWebDriverLevels[index];
 }
 
+// Returns WD wire protocol level name for a WebDriverLog::WebDriverLevel.
 std::string GetWebDriverLevelName(
     const WebDriverLog::WebDriverLevel level) {
   const int index = level - WebDriverLog::kWdAll;
-  CHECK(index >= 0);
-  CHECK(static_cast<size_t>(index) < arraysize(kWebDriverLevelNames));
+  CHECK_GE(index, 0);
+  CHECK_LT(static_cast<size_t>(index), arraysize(kWebDriverLevelNames));
   return kWebDriverLevelNames[index];
 }
 
 }  // namespace
 
 bool WebDriverLog::NameToLevel(
     const std::string& name, WebDriverLog::WebDriverLevel* out_level) {
   for (size_t i = 0; i < arraysize(kWebDriverLevelNames); ++i) {
     if (name == kWebDriverLevelNames[i]) {
       CHECK(WebDriverLog::kWdAll + i <= WebDriverLog::kWdOff);

[jdennett, 2013/05/23 03:44:24]

CHECK_LE

[klm, 2013/05/23 15:21:39]

Done.

       *out_level =
           static_cast<WebDriverLog::WebDriverLevel>(WebDriverLog::kWdAll + i);
       return true;
     }
   }
   return false;
 }
 
 WebDriverLog::WebDriverLog(
     const std::string& type, WebDriverLog::WebDriverLevel min_wd_level)
     : type_(type),
       min_wd_level_(min_wd_level),
       entries_(new base::ListValue()) {
   VLOG(1) << "Log(" << type_ << ", " << min_wd_level_ << ")";
 }
 
 WebDriverLog::~WebDriverLog() {
   VLOG(1) << "Log type '" << type_ << "' lost "
-      << entries_->GetSize() << " entries on destruction";
+          << entries_->GetSize() << " entries on destruction";
 }
 
 const std::string& WebDriverLog::GetType() {
   return type_;
 }
 
 void WebDriverLog::AddEntry(const base::Time& time,
                             Log::Level level,
                             const std::string& message) {
   const WebDriverLog::WebDriverLevel wd_level = LogLevelToWebDriverLevel(level);
@@ skipping 54 matching lines @@
   WebDriverLog* browser_log = new WebDriverLog("browser", browser_log_level);
   devtools_logs.push_back(browser_log);
   // If the level is OFF, don't even bother listening for DevTools events.
   if (browser_log_level != WebDriverLog::kWdOff)
     listeners.push_back(new ConsoleLogger(browser_log));
 
   out_devtools_logs->swap(devtools_logs);
   out_listeners->swap(listeners);
   return Status(kOk);
 }