Fix "unreachable code" warnings (MSVC warning 4702) in content/.

content/common/indexed_db/indexed_db_key.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 "content/common/indexed_db/indexed_db_key.h"
 
 #include <string>
 #include "base/logging.h"
 
 namespace content {
@@ skipping 103 matching lines @@
     return type_ > other.type_ ? -1 : 1;
 
   switch (type_) {
     case WebIDBKeyTypeArray:
       for (size_t i = 0; i < array_.size() && i < other.array_.size(); ++i) {
         if (int result = array_[i].Compare(other.array_[i]))
           return result;
       }
       if (array_.size() < other.array_.size())
         return -1;
-      if (array_.size() > other.array_.size())
-        return 1;
-      return 0;
+      return (array_.size() > other.array_.size()) ? 1 : 0;

[jam, 2014/03/19 16:44:36]

here, and in line 133 and 138, leave the code as is

[jsbell, 2014/03/19 16:52:05]

I wouldn't object to a follow-up patch that introduced a helper:

namespace {
template<typename T>
Compare(const T& a, const T& b) {
  if (a < b) return -1;
  return (b < a) ? 1 : 0;
}
}

[Peter Kasting, 2014/03/19 17:50:42]

133 and 138 nest ternaries, which has always been informally banned because
people don't find it readable.  (I know this because the ban was instituted
because I tried to do this a few times in the early days.)  The code on these
lines was fine, but if it's going to do the same thing as multiple other cases,
shouldn't it look the same as those other cases?

I'm fine with doing it as a separate change, but I think this really should be
cleaned up.  Do you like jsbell's helper solution?  To me it's kind of overkill
and I'd prefer doing things as I did them here, but I'd like to know what you
prefer since you own the code.

[Peter Kasting, 2014/03/19 21:10:57]

(Splitting this to a separate CL so this CL doesn't make changes in this area)

+

[jam, 2014/03/19 16:44:36]

ditto

[Peter Kasting, 2014/03/19 21:10:57]

Done.

     case WebIDBKeyTypeBinary:
       return binary_.compare(other.binary_);
+
     case WebIDBKeyTypeString:
       return string_.compare(other.string_);
+
     case WebIDBKeyTypeDate:
-      return (date_ < other.date_) ? -1 : (date_ > other.date_) ? 1 : 0;
+      if (date_ < other.date_)
+        return -1;
+      return (date_ > other.date_) ? 1 : 0;
+
     case WebIDBKeyTypeNumber:
-      return (number_ < other.number_) ? -1 : (number_ > other.number_) ? 1 : 0;
+      if (number_ < other.number_)
+        return -1;
+      return (number_ > other.number_) ? 1 : 0;
+
     case WebIDBKeyTypeInvalid:
     case WebIDBKeyTypeNull:
     case WebIDBKeyTypeMin:
     default:
       NOTREACHED();
       return 0;
   }
-  NOTREACHED();
-  return 0;
 }
 
 bool IndexedDBKey::IsLessThan(const IndexedDBKey& other) const {
   return Compare(other) < 0;
 }
 
 bool IndexedDBKey::IsEqual(const IndexedDBKey& other) const {
   return !Compare(other);
 }
 
 bool IndexedDBKey::IsValid() const {
   if (type_ == WebIDBKeyTypeInvalid || type_ == WebIDBKeyTypeNull)
     return false;
 
   if (type_ == WebIDBKeyTypeArray) {
     for (size_t i = 0; i < array_.size(); i++) {
       if (!array_[i].IsValid())
         return false;
     }
   }
 
   return true;
 }
 
 }  // namespace content