Add SampleMap and use it in SparseHistogram

base/metrics/sample_map.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.
+
+// SampleMap implements HistogramSamples interface. It is used by the
+// SparseHistogram class to store samples.
+
+#ifndef BASE_METRICS_SAMPLE_MAP_H_
+#define BASE_METRICS_SAMPLE_MAP_H_
+
+#include <map>
+
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_samples.h"
+
+namespace base {
+
+class BASE_EXPORT_PRIVATE SampleMap : public HistogramSamples {
+ public:
+  SampleMap();
+  virtual ~SampleMap();
+
+  // HistogramSamples implementation:
+  virtual void Accumulate(HistogramBase::Sample value,
+                          HistogramBase::Count count) OVERRIDE;
+  virtual HistogramBase::Count GetCount(
+      HistogramBase::Sample value) const OVERRIDE;
+  virtual HistogramBase::Count TotalCount() const OVERRIDE;
+  virtual scoped_ptr<SampleCountIterator> Iterator() const OVERRIDE;
+
+  void ResetRedundantCount(HistogramBase::Count count);
+
+ protected:
+  virtual bool AddSubtractImpl(
+      SampleCountIterator* iter,
+      HistogramSamples::Instruction instruction) OVERRIDE;

[jar (doing other things), 2012/10/05 01:27:36]

The variable name and type bothered me (made me think too much when reading
this).  You should consider changing the name (but I'm not coming up with great
improvements). 

Perhaps:
HistogtramSamples::Operator operator

You suggested a comment, that "|instruction| is either ADD or SUBTRACT" which
probably should go above the declaration to add clarity.

[kaiwang, 2012/10/05 03:16:17]

Changed to "Operator op", because operator is C++ key word

+
+ private:
+  std::map<HistogramBase::Sample, HistogramBase::Count> sample_counts_;
+
+  DISALLOW_COPY_AND_ASSIGN(SampleMap);
+};
+
+class BASE_EXPORT_PRIVATE SampleMapIterator : public SampleCountIterator {
+ public:
+  SampleMapIterator(
+      const std::map<HistogramBase::Sample, HistogramBase::Count>&
+          sample_counts);

[jar (doing other things), 2012/10/05 01:27:36]

This is a looong line :-/.  I really think typdefs would help.

At least add a typedef to this class, so that it helps with lines 49, 61, and
62.

Possible example:
SampleToCountMap

[kaiwang, 2012/10/05 03:16:17]

Done.

+
+  virtual ~SampleMapIterator();
+
+  // SampleCountIterator implementation:
+  virtual bool Done() const OVERRIDE;
+  virtual void Next() OVERRIDE;
+  virtual void Get(HistogramBase::Sample* min,
+                   HistogramBase::Sample* max,
+                   HistogramBase::Count* count) const OVERRIDE;
+ private:
+  std::map<HistogramBase::Sample, HistogramBase::Count>::const_iterator iter_;
+  const std::map<HistogramBase::Sample, HistogramBase::Count>::const_iterator
+      end_;
+};
+
+}  // namespace base
+
+#endif  // BASE_METRICS_SAMPLE_MAP_H_