Address review comments

elastic · Jun 12, 2018 · 2181d41 · 2181d41
1 parent ec1db1a
commit 2181d41
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 31 deletions.
diff --git a/include/model/CDetectionRule.h b/include/model/CDetectionRule.h
@@ -37,10 +37,6 @@ class MODEL_EXPORT CDetectionRule {
     enum ERuleAction { E_SkipResult = 1, E_SkipModelUpdate = 2 };
 
 public:
-    //! Default constructor.
-    //! The rule's action defaults to SKIP_RESULT.
-    CDetectionRule();
-
     //! Set the rule's action.
     void action(int ruleAction);
 
@@ -73,7 +69,7 @@ class MODEL_EXPORT CDetectionRule {
     //! The rule action. It works as a bit mask so its value
     //! may not match any of the declared enum values but the
     //! corresponding bit will be 1 when an action is enabled.
-    int m_Action;
+    int m_Action{E_SkipResult};
 
     //! The rule scope.
     CRuleScope m_Scope;

diff --git a/include/model/CRuleScope.h b/include/model/CRuleScope.h
@@ -41,7 +41,7 @@ class MODEL_EXPORT CRuleScope {
 
 public:
     //! Default constructor.
-    CRuleScope();
+    CRuleScope() = default;
 
     //! Adds a requirement for \p field to be in \p filter for the rule to apply
     void include(std::string field, const core::CPatternSet& filter);

diff --git a/lib/model/CDetectionRule.cc b/lib/model/CDetectionRule.cc
@@ -12,9 +12,6 @@
 namespace ml {
 namespace model {
 
-CDetectionRule::CDetectionRule() : m_Action(E_SkipResult), m_Conditions() {
-}
-
 void CDetectionRule::action(int action) {
     m_Action = action;
 }
@@ -46,8 +43,8 @@ bool CDetectionRule::apply(ERuleAction action,
         return false;
     }
 
-    for (std::size_t i = 0; i < m_Conditions.size(); ++i) {
-        if (m_Conditions[i].test(model, feature, resultType, pid, cid, time) == false) {
+    for (const auto& condition : m_Conditions) {
+        if (condition.test(model, feature, resultType, pid, cid, time) == false) {
             return false;
         }
     }

diff --git a/lib/model/CRuleScope.cc b/lib/model/CRuleScope.cc
@@ -14,21 +14,18 @@
 namespace ml {
 namespace model {
 
-CRuleScope::CRuleScope() : m_Scope() {
-}
-
 void CRuleScope::include(std::string field, const core::CPatternSet& filter) {
-    m_Scope.push_back({field, TPatternSetCRef(filter), E_Include});
+    m_Scope.emplace_back(field, TPatternSetCRef(filter), E_Include);
 }
 
 void CRuleScope::exclude(std::string field, const core::CPatternSet& filter) {
-    m_Scope.push_back({field, TPatternSetCRef(filter), E_Exclude});
+    m_Scope.emplace_back(field, TPatternSetCRef(filter), E_Exclude);
 }
 
 bool CRuleScope::check(const CAnomalyDetectorModel& model, std::size_t pid, std::size_t cid) const {
 
     const CDataGatherer& gatherer = model.dataGatherer();
-    for (auto& scopeField : m_Scope) {
+    for (const auto& scopeField : m_Scope) {
         bool containsValue{false};
         if (scopeField.first == gatherer.partitionFieldName()) {
             containsValue = scopeField.second.get().contains(gatherer.partitionFieldValue());
@@ -51,21 +48,19 @@ bool CRuleScope::check(const CAnomalyDetectorModel& model, std::size_t pid, std:
 
 std::string CRuleScope::print() const {
     std::string result{""};
-    if (m_Scope.empty() == false) {
-        auto itr = m_Scope.begin();
-        while (itr != m_Scope.end()) {
-            result += "'" + itr->first + "'";
-            if (itr->third == E_Include) {
-                result += " IN ";
-            } else {
-                result += " NOT IN ";
-            }
-            result += "FILTER";
+    auto itr = m_Scope.begin();
+    while (itr != m_Scope.end()) {
+        result += "'" + itr->first + "'";
+        if (itr->third == E_Include) {
+            result += " IN ";
+        } else {
+            result += " NOT IN ";
+        }
+        result += "FILTER";
 
-            ++itr;
-            if (itr != m_Scope.end()) {
-                result += " AND ";
-            }
+        ++itr;
+        if (itr != m_Scope.end()) {
+            result += " AND ";
         }
     }
     return result;