From fe46973f26ae2d6505f5dbd60f2850205be53860 Mon Sep 17 00:00:00 2001
From: tsaichristine <tsaichristine@google.com>
Date: Tue, 19 May 2020 14:53:11 -0700
Subject: [PATCH] Clean up ConditionTracker variables for tracking unsliced
 condition

Variables mNonSlicedConditionState and mUnSlicedPart both track the
unsliced condition in ConditionTrackers. However, they are used
inconsistently. These variables can be replaced with one variable and
duplicate logic can be cleaned up.

Bug: b/157079077
Test: m statsd_test && adb sync data && adb shell data/nativetest/statsd_test/statsd_test
Change-Id: I00b577e09a1fd703624eb61eabf2e1c6fee26731
---
 .../condition/CombinationConditionTracker.cpp | 13 +++++-------
 cmds/statsd/src/condition/ConditionTracker.h  | 20 ++++++++++---------
 .../src/condition/SimpleConditionTracker.cpp  | 14 -------------
 3 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/cmds/statsd/src/condition/CombinationConditionTracker.cpp b/cmds/statsd/src/condition/CombinationConditionTracker.cpp
index 2d7f912dac846..c829ccd11ec95 100644
--- a/cmds/statsd/src/condition/CombinationConditionTracker.cpp
+++ b/cmds/statsd/src/condition/CombinationConditionTracker.cpp
@@ -141,17 +141,14 @@ void CombinationConditionTracker::evaluateCondition(
     ConditionState newCondition =
             evaluateCombinationCondition(mChildren, mLogicalOperation, nonSlicedConditionCache);
     if (!mSliced) {
+        bool nonSlicedChanged = (mUnSlicedPartCondition != newCondition);
+        mUnSlicedPartCondition = newCondition;
 
-        bool nonSlicedChanged = (mNonSlicedConditionState != newCondition);
-        mNonSlicedConditionState = newCondition;
-
-        nonSlicedConditionCache[mIndex] = mNonSlicedConditionState;
-
+        nonSlicedConditionCache[mIndex] = mUnSlicedPartCondition;
         conditionChangedCache[mIndex] = nonSlicedChanged;
-        mUnSlicedPart = newCondition;
     } else {
-        mUnSlicedPart = evaluateCombinationCondition(
-            mUnSlicedChildren, mLogicalOperation, nonSlicedConditionCache);
+        mUnSlicedPartCondition = evaluateCombinationCondition(mUnSlicedChildren, mLogicalOperation,
+                                                              nonSlicedConditionCache);
 
         for (const int childIndex : mChildren) {
             // If any of the sliced condition in children condition changes, the combination
diff --git a/cmds/statsd/src/condition/ConditionTracker.h b/cmds/statsd/src/condition/ConditionTracker.h
index 26de88860ab40..f9a2c344c3463 100644
--- a/cmds/statsd/src/condition/ConditionTracker.h
+++ b/cmds/statsd/src/condition/ConditionTracker.h
@@ -36,7 +36,7 @@ public:
           mIndex(index),
           mInitialized(false),
           mTrackerIndex(),
-          mNonSlicedConditionState(ConditionState::kUnknown),
+          mUnSlicedPartCondition(ConditionState::kUnknown),
           mSliced(false){};
 
     virtual ~ConditionTracker(){};
@@ -72,11 +72,6 @@ public:
                                    std::vector<ConditionState>& conditionCache,
                                    std::vector<bool>& conditionChanged) = 0;
 
-    // Return the current condition state.
-    virtual ConditionState isConditionMet() const {
-        return mNonSlicedConditionState;
-    };
-
     // Query the condition with parameters.
     // [conditionParameters]: a map from condition name to the HashableDimensionKey to query the
     //                       condition.
@@ -125,8 +120,9 @@ public:
         const std::vector<sp<ConditionTracker>>& allConditions,
         const vector<Matcher>& dimensions) const = 0;
 
+    // Return the current condition state of the unsliced part of the condition.
     inline ConditionState getUnSlicedPartConditionState() const  {
-        return mUnSlicedPart;
+        return mUnSlicedPartCondition;
     }
 
 protected:
@@ -141,10 +137,16 @@ protected:
     // the list of LogMatchingTracker index that this ConditionTracker uses.
     std::set<int> mTrackerIndex;
 
-    ConditionState mNonSlicedConditionState;
+    // This variable is only used for CombinationConditionTrackers.
+    // SimpleConditionTrackers technically don't have an unsliced part because
+    // they are either sliced or unsliced.
+    //
+    // CombinationConditionTrackers have multiple children ConditionTrackers
+    // that can be a mixture of sliced or unsliced. This tracks the
+    // condition of the unsliced part of the combination condition.
+    ConditionState mUnSlicedPartCondition;
 
     bool mSliced;
-    ConditionState mUnSlicedPart;
 };
 
 }  // namespace statsd
diff --git a/cmds/statsd/src/condition/SimpleConditionTracker.cpp b/cmds/statsd/src/condition/SimpleConditionTracker.cpp
index 61760f3e29b2a..f23ec50abb509 100644
--- a/cmds/statsd/src/condition/SimpleConditionTracker.cpp
+++ b/cmds/statsd/src/condition/SimpleConditionTracker.cpp
@@ -85,12 +85,6 @@ SimpleConditionTracker::SimpleConditionTracker(
         mInitialValue = ConditionState::kUnknown;
     }
 
-    mNonSlicedConditionState = mInitialValue;
-
-    if (!mSliced) {
-        mUnSlicedPart = mInitialValue;
-    }
-
     mInitialized = true;
 }
 
@@ -141,9 +135,6 @@ void SimpleConditionTracker::handleStopAll(std::vector<ConditionState>& conditio
     mInitialValue = ConditionState::kFalse;
     mSlicedConditionState.clear();
     conditionCache[mIndex] = ConditionState::kFalse;
-    if (!mSliced) {
-        mUnSlicedPart = ConditionState::kFalse;
-    }
 }
 
 bool SimpleConditionTracker::hitGuardRail(const HashableDimensionKey& newKey) {
@@ -305,9 +296,7 @@ void SimpleConditionTracker::evaluateCondition(
                 conditionCache[mIndex] =
                         itr->second > 0 ? ConditionState::kTrue : ConditionState::kFalse;
             }
-            mUnSlicedPart = conditionCache[mIndex];
         }
-
         return;
     }
 
@@ -333,9 +322,6 @@ void SimpleConditionTracker::evaluateCondition(
     }
     conditionCache[mIndex] = overallState;
     conditionChangedCache[mIndex] = overallChanged;
-    if (!mSliced) {
-        mUnSlicedPart = overallState;
-    }
 }
 
 void SimpleConditionTracker::isConditionMet(