Create StringOrdinal to allow placement of strings in sorted lists

chrome/common/string_ordinal.h

Index: chrome/common/string_ordinal.h
diff --git a/chrome/common/string_ordinal.h b/chrome/common/string_ordinal.h
new file mode 100644
index 0000000000000000000000000000000000000000..d37b4afcaef293fef3ce0021665631e0b63c5527
--- /dev/null
+++ b/chrome/common/string_ordinal.h
@@ -0,0 +1,66 @@
+// Copyright (c) 2011 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.
+
+#ifndef CHROME_COMMON_STRING_ORDINAL_H_
+#define CHROME_COMMON_STRING_ORDINAL_H_
+#pragma once
+
+#include <string>
+
+// A StringOrdinal represents a specially-formatted string that can be used
+// for ordering. The StringOrdinal class has an unbounded dense strict total
+// order, which mean for any StringOrdinals a, b and c:
+// - a < b and b < c implies a < c (transitivity);

[akalin, 2011/10/20 05:25:49]

add extra line before, and indent each bullet 2 spaces.  So:

// ...a, b, and c:
//
//   - a < b and ..
//   - ...
//   - there are ...
//
// This...

+// - exactly one of a < b, b < a and a = b holds (trichotomy);
+// - if a < b, there is a StringOrdinal x such that a < x < b (density);
+// - there are StringOrdinals x and y such that x < a < y (unboundedness).
+//
+// This means that when StringOrdinal is used for sorting a list, if
+// any item changes its position in the list, only its StringOrinal value
+// has to change to represent the new order, and all the other older values
+// can stay the same.
+class StringOrdinal {
+ public:
+  // Create a StringOrdinal from the given string. It may be valid or invalid.
+  explicit StringOrdinal(const std::string& string_ordinal);
+
+  // Create an invalid StringOrdinal.
+  StringOrdinal();

[tfarina, 2011/10/20 13:02:36]

I think it would be less confusing to have an Invalid() static function that
returns StringOrdinal for is_valid_ equal false.

[akalin, 2011/10/20 18:44:30]

I disagree.  Negative tests are confusing.

+
+  bool IsValid() const;
+
+  // All remaining functions can only be called if IsValid() holds.
+  // It is an error to call them if IsValid() is false.
+
+  // Ordering Functions
+
+  // Returns true iff |*this| < |other|.
+  bool LessThan(const StringOrdinal& other) const;
+
+  // Return true iff |*this| == |other| (i.e. this->LessThan(other) and
+  // other.LessThan(*this) are both false).
+  bool Equal(const StringOrdinal& other) const;
+
+  // Given |*this| != |other|, returns a StringOrdinal x such that
+  // min(|*this|, |other|) < x < max(|*this|,|other|). It is an error

[akalin, 2011/10/20 05:25:49]

space after comma

+  // to call this function with |*this| == |other|.
+  StringOrdinal CreateBetween(const StringOrdinal& other) const;
+
+  // Returns a StringOrdinal x such that x < |*this|.
+  StringOrdinal CreateBefore() const;
+
+  // Returns a StringOrdinal x such that |*this| < x.
+  StringOrdinal CreateAfter() const;
+
+  // It is guaranteed that a StringOrdinal constructed from the returned
+  // string will be valid.
+  std::string ToString() const;
+
+  // Use of copy constructor and default assignment for this class is allowed.

[tfarina, 2011/10/20 13:02:36]

Please, move this to the end of the private section, where we normally place
DISALLOW_COPY_AND_ASSIGN macro.

[akalin, 2011/10/20 18:44:30]

I disagree.  This is part of the public interface.

+
+ private:
+  std::string string_ordinal_;

[tfarina, 2011/10/20 13:02:36]

Can you document this two vars?

+  bool is_valid_;

[tfarina, 2011/10/20 13:02:36]

can you make this const?

[akalin, 2011/10/20 18:44:30]

Can't, to keep it assignable.

+};
+#endif  // CHROME_COMMON_STRING_ORDINAL_H_

[tfarina, 2011/10/20 13:02:36]

please add a blank line above.