Clean up AccumulatorAvg

Major change is not inheriting from AccumulatorSum

Prep for SERVER-5044 $stdDev accumulator

3 files changed, 24 insertions, 31 deletions

diff --git a/src/mongo/db/pipeline/accumulator.h b/src/mongo/db/pipeline/accumulator.h
index 5d6bac52ad4..61b2dade4e7 100644
--- a/src/mongo/db/pipeline/accumulator.h
+++ b/src/mongo/db/pipeline/accumulator.h
@@ -129,14 +129,12 @@ namespace mongo {
 
         static intrusive_ptr<Accumulator> create();
 
-    protected: /* reused by AccumulatorAvg */
+    private:
         AccumulatorSum();
 
         BSONType totalType;
         long long longTotal;
         double doubleTotal;
-        // count is only used by AccumulatorAvg, but lives here to avoid counting non-numeric values
-        long long count;
     };
 
 
@@ -174,8 +172,7 @@ namespace mongo {
     };
 
 
-    class AccumulatorAvg : public AccumulatorSum {
-        typedef AccumulatorSum Super;
+    class AccumulatorAvg : public Accumulator {
     public:
         virtual void processInternal(const Value& input, bool merging);
         virtual Value getValue(bool toBeMerged) const;
@@ -186,5 +183,8 @@ namespace mongo {
 
     private:
         AccumulatorAvg();
+
+        double _total;
+        long long _count;
     };
 }
diff --git a/src/mongo/db/pipeline/accumulator_avg.cpp b/src/mongo/db/pipeline/accumulator_avg.cpp
index dc0ae953ef5..8dbfc816de1 100644
--- a/src/mongo/db/pipeline/accumulator_avg.cpp
+++ b/src/mongo/db/pipeline/accumulator_avg.cpp
@@ -42,20 +42,19 @@ namespace {
 
     void AccumulatorAvg::processInternal(const Value& input, bool merging) {
         if (!merging) {
-            Super::processInternal(input, merging);
+            // non numeric types have no impact on average
+            if (!input.numeric())
+                return;
+
+            _total += input.getDouble();
+            _count += 1;
         }
         else {
             // We expect an object that contains both a subtotal and a count.
             // This is what getValue(true) produced below.
             verify(input.getType() == Object);
-
-            Value subTotal = input[subTotalName];
-            verify(!subTotal.missing());
-            doubleTotal += subTotal.getDouble();
-                
-            Value subCount = input[countName];
-            verify(!subCount.missing());
-            count += subCount.getLong();
+            _total += input[subTotalName].getDouble();
+            _count += input[countName].getLong();
         }
     }
 
@@ -65,32 +64,30 @@ namespace {
 
     Value AccumulatorAvg::getValue(bool toBeMerged) const {
         if (!toBeMerged) {
-            double avg = 0;
-            if (count)
-                avg = doubleTotal / static_cast<double>(count);
+            if (_count == 0)
+                return Value(0.0);
 
-            return Value(avg);
+            return Value(_total / static_cast<double>(_count));
         }
         else {
-            MutableDocument out;
-            out.addField(subTotalName, Value(doubleTotal));
-            out.addField(countName, Value(count));
-
-            return Value(out.freeze());
+            return Value(DOC(subTotalName << _total
+                          << countName << _count));
         }
     }
 
-    AccumulatorAvg::AccumulatorAvg() {
+    AccumulatorAvg::AccumulatorAvg()
+        : _total(0)
+        , _count(0)
+    {
         // This is a fixed size Accumulator so we never need to update this
         _memUsageBytes = sizeof(*this);
     }
 
     void AccumulatorAvg::reset() {
-        // All state is in parent
-        Super::reset();
+        _total = 0;
+        _count = 0;
     }
 
-
     const char *AccumulatorAvg::getOpName() const {
         return "$avg";
     }
diff --git a/src/mongo/db/pipeline/accumulator_sum.cpp b/src/mongo/db/pipeline/accumulator_sum.cpp
index 8595bae0177..76bddc956fe 100644
--- a/src/mongo/db/pipeline/accumulator_sum.cpp
+++ b/src/mongo/db/pipeline/accumulator_sum.cpp
@@ -54,8 +54,6 @@ namespace mongo {
             // non numerics should have returned above so we should never get here
             verify(false);
         }
-
-        count++;
     }
 
     intrusive_ptr<Accumulator> AccumulatorSum::create() {
@@ -81,7 +79,6 @@ namespace mongo {
         : totalType(NumberInt)
         , longTotal(0)
         , doubleTotal(0)
-        , count(0)
     {
         // This is a fixed size Accumulator so we never need to update this
         _memUsageBytes = sizeof(*this);
@@ -91,7 +88,6 @@ namespace mongo {
         totalType = NumberInt;
         longTotal = 0;
         doubleTotal = 0;
-        count = 0;
     }
 
     const char *AccumulatorSum::getOpName() const {