[#3253] addresed review

Author: Razvan Becheriu

src/hooks/dhcp/perfmon/alarm_store.cc

@@ -48,8 +48,8 @@ AlarmStore::checkDurationSample(DurationKeyPtr key, const Duration& sample,
     auto& index = alarms_.get<AlarmPrimaryKeyTag>();
     auto alarm_iter = index.find(*key);
 
-    // If we find an alarm the check the sample.  Alarm::checkSample()
-    //does not alter the key so it can be done in-place.
+    // If we find an alarm then we check the sample.  Alarm::checkSample()
+    // does not alter the key so it can be done in-place.
     if (alarm_iter != index.end()) {
         bool should_report = false;
         bool modified = index.modify(alarm_iter,
@@ -87,7 +87,7 @@ AlarmStore::addAlarm(AlarmPtr alarm) {
 
 AlarmPtr
 AlarmStore::addAlarm(DurationKeyPtr key, const Duration& low_water,
-                      const Duration& high_water, bool enabled /* = true */) {
+                     const Duration& high_water, bool enabled /* = true */) {
     validateKey("addAlarm", key);
 
     // Create the alarm instance.
@@ -149,8 +149,8 @@ AlarmStore::getAll() {
     MultiThreadingLock lock(*mutex_);
     const auto& index = alarms_.get<AlarmPrimaryKeyTag>();
     AlarmCollectionPtr collection(new AlarmCollection());
-    for (auto alarm_iter = index.begin(); alarm_iter != index.end(); ++alarm_iter) {
-        collection->push_back(AlarmPtr(new Alarm(**alarm_iter)));
+    for (auto const& alarm : index) {
+        collection->push_back(AlarmPtr(new Alarm(*alarm)));
     }
 
     return (collection);

src/hooks/dhcp/perfmon/alarm_store.h

@@ -36,7 +36,7 @@ class DuplicateAlarm : public Exception {
 /// @brief Tag for index by primary key (DurationKey).
 struct AlarmPrimaryKeyTag { };
 
-/// @brief A multi index container holding pointers to Alarms.
+/// @brief A multi index container holding pointers to alarms.
 ///
 /// The alarms in the container may be accessed using different indexes:
 /// - using the index on DurationKey members, AlarmPrimaryKeyTag
@@ -64,10 +64,10 @@ typedef std::vector<AlarmPtr> AlarmCollection;
 /// @brief Type for a pointer to a collection of AlarmPtrs.
 typedef boost::shared_ptr<AlarmCollection> AlarmCollectionPtr;
 
-/// @brief Maintains an in-memory store of Alarms
+/// @brief Maintains an in-memory store of alarms
 ///
 /// Provides essential CRUD functions for managing a collection of
-/// Alarms.  Additionally there are finders that can return
+/// alarms.  Additionally there are finders that can return
 /// durations by DurationKey (others are TBD).
 /// All finders return copies of the durations found, rather than the
 /// stored duration itself.
@@ -102,22 +102,22 @@ class AlarmStore {
     AlarmPtr checkDurationSample(DurationKeyPtr key, const Duration& sample,
                                  const Duration& report_interval);
 
-    /// @brief Creates a new Alarm and adds it to the store
+    /// @brief Creates a new alarm and adds it to the store
     ///
-    /// @param key key value of the Alarm to create.
+    /// @param key key value of the alarm to create.
     /// @param low_water threshold below which the average duration must fall to clear the alarm
     /// @param high_water threshold above which the average duration must rise to trigger the alarm.
     /// @param enabled true sets state to CLEAR, otherwise DISABLED, defaults to true.
     ///
-    /// @return pointer to the newly created Alarm.
+    /// @return pointer to the newly created alarm.
     /// @throw DuplicateAlarm if a duration for the given key already exists in
     /// the store.
     AlarmPtr addAlarm(DurationKeyPtr key, const Duration& low_water,
                       const Duration& high_water, bool enabled = true);
 
-    /// @brief Adds an Alarm  to the store.
+    /// @brief Adds an alarm to the store.
     ///
-    /// @return pointer to a copy of the Alarm added.
+    /// @return pointer to a copy of the alarm added.
     AlarmPtr addAlarm(AlarmPtr alarm);
 
     /// @brief Fetches a duration from the store for a given key.
@@ -162,7 +162,7 @@ class AlarmStore {
     /// @brief Convenience method to verify a key is valid for an operation.
     ///
     /// @param label description of where the check is being made, appears in exception text.
-    /// @param key key to validate
+    /// @param key key to validate.
     ///
     /// @throw BadValue if the key is either empty or its family does not
     /// match the store.

src/hooks/dhcp/perfmon/monitored_duration.cc

@@ -23,7 +23,10 @@ namespace perfmon {
 DurationDataInterval::DurationDataInterval(const Timestamp& start_time /* = PktEvent::now()*/)
     : start_time_(start_time), occurrences_(0),
       min_duration_(pos_infin), max_duration_(neg_infin),
-      total_duration_(microseconds(0)) { } void
+      total_duration_(microseconds(0)) {
+}
+
+void
 DurationDataInterval::addDuration(const Duration& duration) {
     ++occurrences_;
     if (duration < min_duration_) {
@@ -48,13 +51,11 @@ DurationDataInterval::getAverageDuration() const {
 
 bool
 DurationDataInterval::operator==(const DurationDataInterval& other) const {
-    return (
-        (start_time_ == other.start_time_) &&
-        (occurrences_ == other.occurrences_) &&
-        (min_duration_ == other.min_duration_) &&
-        (max_duration_ == other.max_duration_) &&
-        (total_duration_ == other.total_duration_)
-   );
+    return ((start_time_ == other.start_time_) &&
+            (occurrences_ == other.occurrences_) &&
+            (min_duration_ == other.min_duration_) &&
+            (max_duration_ == other.max_duration_) &&
+            (total_duration_ == other.total_duration_));
 }
 
 // DurationKey methods
@@ -190,13 +191,11 @@ DurationKey::operator!=(const DurationKey& other) const {
 
 bool
 DurationKey::operator<(const DurationKey& other) const {
-    return (
-        (query_type_ < other.query_type_) ||
-        (response_type_ < other.response_type_) ||
-        (start_event_label_ < other.start_event_label_) ||
-        (stop_event_label_ < other.stop_event_label_) ||
-        (subnet_id_ < other.subnet_id_)
-    );
+    return ((query_type_ < other.query_type_) ||
+            (response_type_ < other.response_type_) ||
+            (start_event_label_ < other.start_event_label_) ||
+            (stop_event_label_ < other.stop_event_label_) ||
+            (subnet_id_ < other.subnet_id_));
 }
 
 
@@ -237,11 +236,11 @@ MonitoredDuration::MonitoredDuration(const MonitoredDuration& rhs)
       current_interval_(0),
       previous_interval_(0) {
     if (rhs.current_interval_) {
-      current_interval_.reset(new DurationDataInterval(*rhs.current_interval_));
+        current_interval_.reset(new DurationDataInterval(*rhs.current_interval_));
     }
 
     if (rhs.previous_interval_) {
-      previous_interval_.reset(new DurationDataInterval(*rhs.previous_interval_));
+        previous_interval_.reset(new DurationDataInterval(*rhs.previous_interval_));
     }
 }
 

src/hooks/dhcp/perfmon/monitored_duration.h

@@ -237,7 +237,7 @@ class DurationKey {
     ///
     /// less than operator to compare two DurationKey objects.
     /// @param other DurationKey to be compared against.
-    /// @return True other is less than this key
+    /// @return True key is less than the other key
     bool operator<(const DurationKey& other) const;
 
 protected:

src/hooks/dhcp/perfmon/monitored_duration_store.cc

@@ -59,7 +59,7 @@ MonitoredDurationStore::addDurationSample(DurationKeyPtr key, const Duration& sa
         bool should_report = false;
         // Modify updates in place and only re-indexes if keys change.
         bool modified = index.modify(duration_iter,
-                               [sample, &should_report](MonitoredDurationPtr mond){
+                                     [sample, &should_report](MonitoredDurationPtr mond) {
             should_report = mond->addSample(sample);
         });
 
@@ -126,7 +126,7 @@ MonitoredDurationStore::getDuration(DurationKeyPtr key) {
     const auto& index = durations_.get<DurationKeyTag>();
     auto duration_iter = index.find(*key);
     return (duration_iter == index.end() ? MonitoredDurationPtr()
-            : MonitoredDurationPtr(new MonitoredDuration(**duration_iter)));
+                                         : MonitoredDurationPtr(new MonitoredDuration(**duration_iter)));
 }
 
 void
@@ -166,8 +166,8 @@ MonitoredDurationStore::getAll() {
     MultiThreadingLock lock(*mutex_);
     const auto& index = durations_.get<DurationKeyTag>();
     MonitoredDurationCollectionPtr collection(new MonitoredDurationCollection());
-    for (auto duration_iter = index.begin(); duration_iter != index.end(); ++duration_iter) {
-        collection->push_back(MonitoredDurationPtr(new MonitoredDuration(**duration_iter)));
+    for (auto const& mond : index) {
+        collection->push_back(MonitoredDurationPtr(new MonitoredDuration(*mond)));
     }
 
     return (collection);
@@ -186,7 +186,7 @@ MonitoredDurationStore::getReportsNext() {
     // We want to find the oldest interval that is less than interval_duration in the past.
     auto duration_iter = index.lower_bound(dhcp::PktEvent::now() - interval_duration_);
     return (duration_iter == index.end() ? MonitoredDurationPtr()
-            : MonitoredDurationPtr(new MonitoredDuration(**duration_iter)));
+                                         : MonitoredDurationPtr(new MonitoredDuration(**duration_iter)));
 }
 
 MonitoredDurationCollectionPtr
@@ -195,7 +195,7 @@ MonitoredDurationStore::getOverdueReports(const Timestamp& since /* = PktEvent::
     // We use a lower bound of MIN + 1us to avoid dormant durations
     static Timestamp lower_limit_time(PktEvent::MIN_TIME() + microseconds(1));
 
-    // We want to find anything since the start of time that who's start time
+    // We want to find anything since the start of time who's start time
     // is more than interval_duration_ in the past.
     const auto& index = durations_.get<IntervalStartTag>();
     auto lower_limit = index.lower_bound(lower_limit_time);

src/hooks/dhcp/perfmon/monitored_duration_store.h

@@ -39,7 +39,7 @@ struct DurationKeyTag { };
 /// @brief Tag for index by interval start time.
 struct IntervalStartTag { };
 
-/// @brief A multi index container holding pointers to MonitoredDurations.
+/// @brief A multi index container holding pointers to durations.
 ///
 /// The durations in the container may be accessed using different indexes:
 /// - using the index on DurationKey members, DurationKeyTag
@@ -75,10 +75,10 @@ typedef std::vector<MonitoredDurationPtr> MonitoredDurationCollection;
 /// @brief Type for a pointer to a collection of MonitoredDurationPtrs.
 typedef boost::shared_ptr<MonitoredDurationCollection> MonitoredDurationCollectionPtr;
 
-/// @brief Maintains an in-memory store of MonitoredDurations
+/// @brief Maintains an in-memory store of durations
 ///
 /// Provides essential CRUD functions for managing a collection of
-/// MonitoredDurations.  Additionally there are finders that can return
+/// durations.  Additionally there are finders that can return
 /// durations by DurationKey (others are TBD)
 /// All finders return copies of the durations found, rather than the
 /// stored duration itself.
@@ -100,7 +100,7 @@ class MonitoredDurationStore {
     /// condition, then a copy of the in-store duration is returned, otherwise an empty
     /// pointer is returned.
     ///
-    /// If The duration does not exist in the store, then one is created and inserted
+    /// If the duration does not exist in the store, then one is created and inserted
     /// into the store after adding the sample.  An empty pointer is returned.
     ///
     /// This function does not/must not modify any index keys.
@@ -112,7 +112,7 @@ class MonitoredDurationStore {
     /// pointer otherwise.
     MonitoredDurationPtr addDurationSample(DurationKeyPtr key, const Duration& sample);
 
-    /// @brief Creates a new MonitoredDuration and adds it to the store
+    /// @brief Creates a new duration and adds it to the store
     ///
     /// @param key key value of the duration to create.
     ///
@@ -134,7 +134,7 @@ class MonitoredDurationStore {
     ///
     /// @param duration duration to update.
     ///
-    /// @throw InvalidOperation if MonitoredDuration does not exist in the store.
+    /// @throw InvalidOperation if duration does not exist in the store.
     void updateDuration(MonitoredDurationPtr& duration);
 
     /// @brief Removes the duration from the store.
@@ -161,8 +161,7 @@ class MonitoredDurationStore {
     ///
     /// @return a collection of the matching durations, ordered by current interval
     /// start time.
-    MonitoredDurationCollectionPtr getOverdueReports(const Timestamp& since
-                                                     = dhcp::PktEvent::now());
+    MonitoredDurationCollectionPtr getOverdueReports(const Timestamp& since = dhcp::PktEvent::now());
 
     /// @brief Removes all durations from the store.
     void clear();
@@ -178,7 +177,7 @@ class MonitoredDurationStore {
     /// @brief Convenience method to verify a key is valid for an operation.
     ///
     /// @param label description of where the check is being made, appears in exception text.
-    /// @param key key to validate
+    /// @param key key to validate.
     ///
     /// @throw BadValue if the key is either empty or its family does not
     /// match the store.
@@ -187,7 +186,7 @@ class MonitoredDurationStore {
     /// @brief Protocol family AF_INET or AF_INET6.
     uint16_t family_;
 
-    /// @brief The length of time over data for a single MonitoredDuration is
+    /// @brief The length of time over data for a single duration is
     /// accumulated before reporting.
     Duration interval_duration_;