Structure find_by_* calls to avoid raw loops

This set of changes is made as preparation for the addition of the Aviso
attribute, and is expected to reduce the duplication of source code.

Re ECFLOW-1931

Author: marcosbento

ACore/src/ecflow/core/Stl.hpp

@@ -57,6 +57,49 @@ void AssoDeletePtrs(Container& pContainer) {
     std::for_each(pContainer.begin(), pContainer.end(), TAsoDeletor<typename Container::value_type>());
     pContainer.clear();
 }
+
+namespace algorithm {
+
+namespace detail {
+
+template <typename T>
+struct is_shared_pointer : std::false_type
+{
+};
+
+template <typename T>
+struct is_shared_pointer<std::shared_ptr<T>> : std::true_type
+{
+};
+
+} // namespace detail
+
+template <typename T>
+constexpr bool is_shared_pointer_v = detail::is_shared_pointer<T>::value;
+
+template <typename C, typename Predicate>
+inline auto find_by(C& container, Predicate predicate) {
+    return std::find_if(std::begin(container), std::end(container), predicate);
+}
+
+template <typename C>
+inline auto find_by_name(C& container, std::string_view name) {
+    // Important: special handling to seamlessly handle containers of std::shared_ptr.
+    if constexpr (is_shared_pointer_v<typename C::value_type>) {
+        return find_by(container, [&](const auto& item) { return item->name() == name; });
+    }
+    else {
+        return find_by(container, [&](const auto& item) { return item.name() == name; });
+    }
+}
+
+template <typename C, typename I>
+inline auto find_by_number(C& container, I number) {
+    return find_by(container, [&](const auto& item) { return item.number() == number; });
+}
+
+} // namespace algorithm
+
 } // namespace ecf
 
 #endif /* ecflow_core_Stl_HPP */

ANode/src/ecflow/node/Jobs.cpp

@@ -54,12 +54,11 @@ bool Jobs::generate(JobsParam& jobsParam) const {
 
         if (defs_) {
             if (defs_->server().get_state() == SState::RUNNING) {
-                const std::vector<suite_ptr>& suiteVec = defs_->suiteVec();
-                size_t theSize                         = suiteVec.size();
-                for (size_t i = 0; i < theSize; i++) {
-                    // SuiteChanged moved internal to Suite::resolveDependencies. i.e on fast path
-                    // and when suites not begun we save a constructor/destructor calls
-                    (void)suiteVec[i]->resolveDependencies(jobsParam);
+                const std::vector<suite_ptr>& suites = defs_->suiteVec();
+                for (const suite_ptr& suite: suites) {
+                    // SuiteChanged moved into Suite::resolveDependencies.
+                    // This ensures the fast path and when suite are not begun we save a ctor/dtor call
+                    (void)suite->resolveDependencies(jobsParam);
                 }
             }
         }

ANode/src/ecflow/node/Node.cpp

@@ -20,6 +20,7 @@
 #include "ecflow/core/Log.hpp"
 #include "ecflow/core/PrintStyle.hpp"
 #include "ecflow/core/Serialization.hpp"
+#include "ecflow/core/Stl.hpp"
 #include "ecflow/core/Str.hpp"
 #include "ecflow/core/cereal_boost_time.hpp"
 #include "ecflow/node/AbstractObserver.hpp"
@@ -1045,11 +1046,9 @@ void Node::setStateOnly(NState::State newState,
 
     // Record state changes for verification
     if (misc_attrs_) {
-        size_t theSize = misc_attrs_->verifys_.size();
-        for (size_t i = 0; i < theSize; i++) {
-            if (misc_attrs_->verifys_[i].state() == newState) {
-                // cout << "Verify: calendar " << to_simple_string(calendar.date()) << "\n";
-                misc_attrs_->verifys_[i].incrementActual();
+        for (auto& verify : misc_attrs_->verifys_) {
+            if (verify.state() == newState) {
+                verify.incrementActual();
             }
         }
     }
@@ -1092,22 +1091,27 @@ DState::State Node::dstate() const {
 }
 
 bool Node::set_event(const std::string& event_name_or_number) {
-    for (Event& e : events_) {
-        if (e.name_or_number() == event_name_or_number) {
-            e.set_value(true);
-            return true;
-        }
+    auto found = ecf::algorithm::find_by(
+        events_, [&](const auto& item) { return item.name_or_number() == event_name_or_number; });
+
+    if (found == std::end(events_)) {
+        return false;
     }
-    return false;
+
+    found->set_value(true);
+    return true;
 }
+
 bool Node::clear_event(const std::string& event_name_or_number) {
-    for (Event& e : events_) {
-        if (e.name_or_number() == event_name_or_number) {
-            e.set_value(false);
-            return true;
-        }
+    auto found = ecf::algorithm::find_by(
+        events_, [&](const auto& item) { return item.name_or_number() == event_name_or_number; });
+
+    if (found == std::end(events_)) {
+        return false;
     }
-    return false;
+
+    found->set_value(false);
+    return true;
 }
 
 void Node::setRepeatToLastValue() {

ANode/src/ecflow/node/NodeAdd.cpp

@@ -15,6 +15,7 @@
 #include "ecflow/attribute/LateAttr.hpp"
 #include "ecflow/core/Converter.hpp"
 #include "ecflow/core/Ecf.hpp"
+#include "ecflow/core/Stl.hpp"
 #include "ecflow/node/AutoRestoreAttr.hpp"
 #include "ecflow/node/Expression.hpp"
 #include "ecflow/node/Limit.hpp"
@@ -25,18 +26,18 @@ using namespace ecf;
 using namespace std;
 
 bool Node::update_variable(const std::string& name, const std::string& value) {
-    size_t theSize = vars_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        if (vars_[i].name() == name) {
-            // Variable already exist, *UPDATE* its value
-            vars_[i].set_value(value);
-            if (0 == Ecf::debug_level())
-                std::cerr << "Node::addVariable: Variable of name '" << name << "' already exist for node "
-                          << debugNodePath() << " updating with value '" << value << "'\n";
-            return true;
-        }
+    auto found = ecf::algorithm::find_by_name(vars_, name);
+
+    if (found == std::end(vars_)) {
+        return false;
+    }
+
+    found->set_value(value);
+    if (0 == Ecf::debug_level()) {
+        std::cerr << "Node::addVariable: Variable of name '" << name << "' already exist for node " << debugNodePath()
+                  << " updating with value '" << value << "'\n";
     }
-    return false;
+    return true;
 }
 
 void Node::addVariable(const Variable& v) {

ANode/src/ecflow/node/NodeChange.cpp

@@ -13,6 +13,7 @@
 #include "ecflow/attribute/LateAttr.hpp"
 #include "ecflow/core/Converter.hpp"
 #include "ecflow/core/Ecf.hpp"
+#include "ecflow/core/Stl.hpp"
 #include "ecflow/core/Str.hpp"
 #include "ecflow/node/ExprAst.hpp"
 #include "ecflow/node/Limit.hpp"
@@ -22,15 +23,14 @@ using namespace ecf;
 using namespace std;
 
 void Node::changeVariable(const std::string& name, const std::string& value) {
-    size_t theSize = vars_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        if (vars_[i].name() == name) {
-            vars_[i].set_value(value);
-            variable_change_no_ = Ecf::incr_state_change_no();
-            return;
-        }
+    auto found = ecf::algorithm::find_by_name(vars_, name);
+
+    if (found == std::end(vars_)) {
+        throw std::runtime_error("Node::changeVariable: Could not find variable " + name);
     }
-    throw std::runtime_error("Node::changeVariable: Could not find variable " + name);
+
+    found->set_value(value);
+    variable_change_no_ = Ecf::incr_state_change_no();
 }
 
 bool Node::set_event(const std::string& event_name_or_number, bool value) {
@@ -39,23 +39,22 @@ bool Node::set_event(const std::string& event_name_or_number, bool value) {
     }
 
     // find by name first
-    size_t theSize = events_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        if (events_[i].name() == event_name_or_number) {
-            events_[i].set_value(value);
+    {
+        auto found = ecf::algorithm::find_by_name(events_, event_name_or_number);
+        if (found != std::end(events_)) {
+            found->set_value(value);
             return true;
         }
     }
 
     // Test for numeric, and then casting, is ****faster***** than relying on exception alone
     if (event_name_or_number.find_first_of(Str::NUMERIC()) == 0) {
         try {
-            auto eventNumber = ecf::convert_to<int>(event_name_or_number);
-            for (size_t i = 0; i < theSize; i++) {
-                if (events_[i].number() == eventNumber) {
-                    events_[i].set_value(value);
-                    return true;
-                }
+            auto number = ecf::convert_to<int>(event_name_or_number);
+            auto found  = ecf::algorithm::find_by_number(events_, number);
+            if (found != std::end(events_)) {
+                found->set_value(value);
+                return true;
             }
         }
         catch (const ecf::bad_conversion&) {
@@ -70,29 +69,28 @@ bool Node::set_event_used_in_trigger(const std::string& event_name_or_number) {
     }
 
     // find by name first
-    size_t theSize = events_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        if (events_[i].name() == event_name_or_number) {
-            events_[i].usedInTrigger(true);
+    {
+        auto found = ecf::algorithm::find_by_name(events_, event_name_or_number);
+        if (found != std::end(events_)) {
+            found->usedInTrigger(true);
             return true;
         }
     }
 
     // Test for numeric, and then casting, is ****faster***** than relying on exception alone
     if (event_name_or_number.find_first_of(Str::NUMERIC()) == 0) {
         try {
-            auto eventNumber = ecf::convert_to<int>(event_name_or_number);
-            for (size_t i = 0; i < theSize; i++) {
-                if (events_[i].number() == eventNumber) {
-                    events_[i].usedInTrigger(true);
-                    return true;
-                    ;
-                }
+            auto number = ecf::convert_to<int>(event_name_or_number);
+            auto found  = ecf::algorithm::find_by_number(events_, number);
+            if (found != std::end(events_)) {
+                found->usedInTrigger(true);
+                return true;
             }
         }
         catch (const ecf::bad_conversion&) {
         }
     }
+
     return false;
 }
 void Node::changeEvent(const std::string& event_name_or_number, const std::string& setOrClear) {
@@ -116,25 +114,25 @@ void Node::changeEvent(const std::string& event_name_or_number, bool value) {
     throw std::runtime_error("Node::changeEvent: Could not find event " + event_name_or_number);
 }
 
-bool Node::set_meter(const std::string& meter_name, int value) {
-    size_t the_meter_size = meters_.size();
-    for (size_t i = 0; i < the_meter_size; ++i) {
-        if (meters_[i].name() == meter_name) {
-            meters_[i].set_value(value);
-            return true;
-        }
+bool Node::set_meter(const std::string& name, int value) {
+    auto found = ecf::algorithm::find_by_name(meters_, name);
+
+    if (found == std::end(meters_)) {
+        return false;
     }
-    return false;
+
+    found->set_value(value);
+    return true;
 }
-bool Node::set_meter_used_in_trigger(const std::string& meter_name) {
-    size_t the_meter_size = meters_.size();
-    for (size_t i = 0; i < the_meter_size; ++i) {
-        if (meters_[i].name() == meter_name) {
-            meters_[i].usedInTrigger(true);
-            return true;
-        }
+bool Node::set_meter_used_in_trigger(const std::string& name) {
+    auto found = ecf::algorithm::find_by_name(meters_, name);
+
+    if (found == std::end(meters_)) {
+        return false;
     }
-    return false;
+
+    found->usedInTrigger(true);
+    return true;
 }
 void Node::changeMeter(const std::string& meter_name, const std::string& value) {
     int theValue = 0;
@@ -146,21 +144,21 @@ void Node::changeMeter(const std::string& meter_name, const std::string& value)
     }
     changeMeter(meter_name, theValue);
 }
+
 void Node::changeMeter(const std::string& meter_name, int value) {
     if (set_meter(meter_name, value))
         return;
     throw std::runtime_error("Node::changeMeter: Could not find meter " + meter_name);
 }
 
 void Node::changeLabel(const std::string& name, const std::string& value) {
-    size_t theSize = labels_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        if (labels_[i].name() == name) {
-            labels_[i].set_new_value(value);
-            return;
-        }
+    auto found = ecf::algorithm::find_by_name(labels_, name);
+
+    if (found == std::end(labels_)) {
+        throw std::runtime_error("Node::changeLabel: Could not find label " + name);
     }
-    throw std::runtime_error("Node::changeLabel: Could not find label " + name);
+
+    found->set_new_value(value);
 }
 
 void Node::changeTrigger(const std::string& expression) {
@@ -241,30 +239,28 @@ void Node::change_time(const std::string& old, const std::string& new_time) {
     TimeAttr old_attr(TimeSeries::create(old));      // can throw if parse fails
     TimeAttr new_attr(TimeSeries::create(new_time)); // can throw if parse fails
 
-    size_t theSize = times_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        // Dont use '==' since that compares additional state like free_
-        if (times_[i].structureEquals(old_attr)) {
-            times_[i]        = new_attr;
-            state_change_no_ = Ecf::incr_state_change_no();
-            return;
-        }
+    // Don't use '==' since that compares additional state like free_
+    auto found = ecf::algorithm::find_by(times_, [&](const auto& item) { return item.structureEquals(old_attr); });
+
+    if (found == std::end(times_)) {
+        throw std::runtime_error("Node::change_time : Cannot find time attribute: ");
     }
-    throw std::runtime_error("Node::change_time : Cannot find time attribute: ");
+
+    *found           = new_attr;
+    state_change_no_ = Ecf::incr_state_change_no();
 }
 
 void Node::change_today(const std::string& old, const std::string& new_time) {
     TodayAttr old_attr(TimeSeries::create(old));      // can throw if parse fails
     TodayAttr new_attr(TimeSeries::create(new_time)); // can throw if parse fails
 
-    size_t theSize = todays_.size();
-    for (size_t i = 0; i < theSize; i++) {
-        // Dont use '==' since that compares additional state like free_
-        if (todays_[i].structureEquals(old_attr)) {
-            todays_[i]       = new_attr;
-            state_change_no_ = Ecf::incr_state_change_no();
-            return;
-        }
+    // Don't use '==' since that compares additional state like free_
+    auto found = ecf::algorithm::find_by(todays_, [&](const auto& item) { return item.structureEquals(old_attr); });
+
+    if (found == std::end(todays_)) {
+        throw std::runtime_error("Node::change_today : Cannot find time attribute: ");
     }
-    throw std::runtime_error("Node::change_today : Cannot find today attribute: ");
+
+    *found           = new_attr;
+    state_change_no_ = Ecf::incr_state_change_no();
 }