Merge pull request #2 from ahmedakef/apply_skeeto_comments

Apply skeeto comments

Author: ahmedakef

main.cpp

@@ -2,18 +2,24 @@
 #include <sstream>
 #include <vector>
 #include <thread>
+#include <mutex>
+#include <condition_variable>
+#include <fcntl.h>
 #include "modules/summarizer.hpp"
 #include "utils/utils.hpp"
-#include <fcntl.h>
 #include <boost/program_options.hpp>
+#include <atomic>
 namespace po = boost::program_options;
 
 using namespace std;
+mutex continue_m;
+condition_variable continue_cv;
 bool continue_reading = true;
 
 void handle_printing(Summarizer *summarizer, int delay, int precision)
 {
     print_elements(vector<string>{"Count", "Mean", "Min", "Max", "P95", "P99", "\n"}, precision);
+    unique_lock<mutex> lock(continue_m);
     while (true)
     {
         cout << "\r";
@@ -23,31 +29,35 @@ void handle_printing(Summarizer *summarizer, int delay, int precision)
             cout << endl;
             break;
         }
-        this_thread::sleep_for(chrono::seconds(delay));
+        // Atomically releases lock, blocks the current executing thread.
+        // The thread will be unblocked when notify_one() is executed, or when the delay expires.
+        // When unblocked, regardless of the reason, lock is reacquired
+        continue_cv.wait_for(lock, chrono::seconds(delay));
     }
 }
 
 void start(int delay, int precision)
 {
-    int number;
     string line;
     Summarizer summarizer;
 
     thread timer(handle_printing, &summarizer, delay, precision);
 
-    while (continue_reading)
+    while (true)
     {
         if (!getline(cin, line))
         {
             // either an error hapened or we reached EOF
+            lock_guard<mutex> lock(continue_m);
             continue_reading = false;
+            continue_cv.notify_one();
             break;
         }
-        if (!is_number(line) || line == "")
+        auto [number, ok] = is_number(line);
+        if (!ok || line == "")
         {
             continue;
         }
-        stringstream(line) >> number;
         summarizer.add_number(number);
     }
 

makefile

@@ -11,6 +11,7 @@ all:
 # all:
 # 	c++ \
 # 	-std=c++11 \
+# 	-fsanitize=thread -g \
 # 	-pthread \
 # 	-o summarize \
 # 	main.cpp utils/utils.cpp modules/summarizer.cpp \

modules/summarizer.cpp

@@ -12,11 +12,15 @@ Summarizer::Summarizer() : acc(tag::extended_p_square::probabilities = probs) {}
 
 void Summarizer::add_number(double number)
 {
+    std::lock_guard<std::mutex> lock(this->lock);
+
     acc(number);
 }
 
 void Summarizer::print_summary(int precision)
 {
+    std::lock_guard<std::mutex> lock(this->lock);
+
     double count_v = count(acc);
     if (count_v == 0.0){
         print_elements(std::vector<double>{0, 0, 0, 0, 0, 0}, precision);

modules/summarizer.hpp

@@ -1,5 +1,6 @@
 #ifndef Summarizer_H
 #define Summarizer_H
+#include <mutex>
 #include <boost/accumulators/accumulators.hpp>
 #include <boost/accumulators/statistics/stats.hpp>
 #include <boost/accumulators/statistics/count.hpp>
@@ -15,6 +16,8 @@ typedef accumulator_set<double, stats<tag::count, tag::mean, tag::min, tag::max,
 class Summarizer
 {
   accumulator_t acc;
+  std::mutex lock;
+
 
 public:
   Summarizer();

utils/utils.cpp

@@ -22,9 +22,9 @@ void print_elements(const vector<T> &elements, int precision)
 template void print_elements(const vector<double> &elements, int precision);
 template void print_elements(const vector<string> &elements, int precision);
 
-bool is_number(const string &s)
+tuple<double, bool> is_number(const string &s)
 {
     char *end;
-    strtod(s.c_str(), &end);
-    return *end == 0;
+    double r = strtod(s.c_str(), &end);
+    return {r, *end == 0};
 }

utils/utils.hpp

@@ -3,13 +3,14 @@
 
 #include <vector>
 #include <string>
+#include <tuple>
 
 template <typename T>
 void print_element(T &t, int precision);
 
 template <typename T>
 void print_elements(const std::vector<T> &elements, int precision);
 
-bool is_number(const std::string &s);
+std::tuple<double, bool> is_number(const std::string &s);
 
 #endif