SampleSet -> HistogramSamples (will be reused by SparseHistogram)

base/metrics/histogram_samples.h

+// 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.
+
+#ifndef BASE_METRICS_HISTOGRAM_SAMPLES_H_
+#define BASE_METRICS_HISTOGRAM_SAMPLES_H_
+
+#include "base/basictypes.h"
+#include "base/metrics/histogram_base.h"
+#include "base/memory/scoped_ptr.h"
+
+class Pickle;
+class PickleIterator;
+
+namespace base {
+
+class SampleCountIterator;
+
+// HistogramSamples is a container storing all samples of a histogram.
+class BASE_EXPORT HistogramSamples {
+ public:
+  HistogramSamples();
+  virtual ~HistogramSamples();
+
+  virtual void Accumulate(HistogramBase::Sample value,
+                          HistogramBase::Count count) = 0;
+  virtual HistogramBase::Count GetCount(HistogramBase::Sample value) const = 0;
+  virtual HistogramBase::Count TotalCount() const = 0;
+
+  virtual void Add(const HistogramSamples& other);
+
+  // Add from serialized samples.
+  virtual bool AddFromPickle(PickleIterator* iter);
+
+  virtual void Subtract(const HistogramSamples& other);
+
+  virtual scoped_ptr<SampleCountIterator> Iterator() const = 0;
+  virtual bool Serialize(Pickle* pickle) const;
+
+  // Accessor fuctions.
+  int64 sum() const { return sum_; }
+  HistogramBase::Count redundant_count() const { return redundant_count_; }
+
+ protected:
+  // Add/subtract sample counts data from the iterator.
+  // Add samples when |is_add| is true, else subtract samples.
+  virtual bool AddSubtractImpl(SampleCountIterator* iter, bool is_add) = 0;

[Ilya Sherman, 2012/08/29 23:41:27]

Rather than passing a boolean parameter, please use an enum.  For an example,
see
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/autofill/fo...

[kaiwang, 2012/08/30 03:13:22]

As replied in earlier comment, I really don't see a reason to make an enum. It's
not like "RequesetMethod", but always only havs 2 states: either add or
subtract.
If there's no coding style against this, I'd prefer to make the function
signature simpler.

Also comment is added to make it more clear.

[Ilya Sherman, 2012/08/30 07:57:40]

The reason to use an enum rather than a boolean is so that it is clear at the
call site what the code is doing.  Compare the following:

CHECK(AddSubtractImpl(iter, false));
CHECK(AddSubtractImpl(iter, SUBTRACT));

The meaning of |false| in the first line is unclear; you need to jump to the
HistogramSamples header to understand it.  The meaning of |SUBTRACT| in the
second line is much clearer.

This, like many other common practices, is not explicitly listed in the style
guide.  However, Jim and I -- and many other Chromium developers -- prefer this
style.  Please use it here.

[kaiwang, 2012/09/07 01:14:11]

Done.

+
+  void IncreaseSum(int64 diff);

[Ilya Sherman, 2012/08/29 23:41:27]

Optional nit: Perhaps "IncreaseSumBy"?

+  void IncreaseRedundantCount(HistogramBase::Count diff);
+
+ private:
+  int64 sum_;
+
+  // |redundant_count_| helps identify memory corruption. It redundantly stores
+  // the total number of samples accumulated in the histogram. We can compare
+  // this count to the sum of the counts (TotalCount() function), and detect
+  // problems. Note, depending on the implementation of different histogram
+  // types, there might be races during histogram accumulation and snapshotting
+  // that we choose to accept. In this case, the tallies might mismatch even
+  // when no memory corruption has happened.
+  HistogramBase::Count redundant_count_;
+};
+
+class BASE_EXPORT SampleCountIterator {
+ public:
+  virtual ~SampleCountIterator();
+
+  virtual bool Done() const = 0;
+  virtual void Next() = 0;
+
+  // Get the sample and count at current position.
+  // |min| |max| and |count| can be NULL if the value is not of interest.
+  virtual void Get(HistogramBase::Sample* min,
+                   HistogramBase::Sample* max,
+                   HistogramBase::Count* count) const = 0;
+};
+
+}  // namespace base
+
+#endif  // BASE_METRICS_HISTOGRAM_SAMPLES_H_