From 2e5d323b525a9fca621e4d3b790c535b1c32efcf Mon Sep 17 00:00:00 2001
From: KjellKod <hedstrom@kjellkod.cc>
Date: Thu, 19 Dec 2013 23:47:31 -0700
Subject: [PATCH] Input from Gem Chen triggerd review of shared_queue (now made
 clearer) and active.hpp (now all header file)

---
 g2log/src/active.cpp       | 54 --------------------------------------
 g2log/src/active.hpp       | 36 ++++++++++++++++++-------
 g2log/src/shared_queue.hpp | 19 +++++++-------
 3 files changed, 36 insertions(+), 73 deletions(-)
 delete mode 100644 g2log/src/active.cpp

diff --git a/g2log/src/active.cpp b/g2log/src/active.cpp
deleted file mode 100644
index 464dc71..0000000
--- a/g2log/src/active.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-/** ==========================================================================
- * 2010 by KjellKod.cc. This is PUBLIC DOMAIN to use at your own risk and comes
- * with no warranties. This code is yours to share, use and modify with no
- * strings attached and no restrictions or obligations.
- * ============================================================================
- *
- * Example of a Active Object, using C++11 std::thread mechanisms to make it
- * safe for thread communication.
- *
- * This was originally published at http://sites.google.com/site/kjellhedstrom2/active-object-with-cpp0x
- * and inspired from Herb Sutter's C++11 Active Object
- * http://herbsutter.com/2010/07/12/effective-concurrency-prefer-using-active-objects-instead-of-naked-threads
- *
- * Last update 2013-09-25 by Kjell Hedstrom,
- * e-mail: hedstrom at kjellkod dot cc
- * linkedin: http://linkedin.com/se/kjellkod */
-
-
-#include "active.hpp"
-using namespace kjellkod;
-Active::Active() : done_(false) {
-}
-Active::~Active() {
-   send([this] {
-      done_ = true;
-   });
-   thd_.join();
-}
-
-/// Add asynchronously a work-message to queue
-void Active::send(Callback msg_) {
-   mq_.push(msg_);
-}
-
-
-/// Will wait for msgs if queue is empty
-/// A great explanation of how this is done (using Qt's library):
-/// http://doc.qt.nokia.com/stable/qwaitcondition.html
-void Active::run() {
-   while (!done_) {
-      // wait till job is available, then retrieve it and
-      // executes the retrieved job in this thread (background)
-      Callback func;
-      mq_.wait_and_pop(func);
-      func();
-   }
-}
-
-/// Factory: safe construction of object before thread start
-std::unique_ptr<Active> Active::createActive() {
-   std::unique_ptr<Active> aPtr(new Active());
-   aPtr->thd_ = std::thread(&Active::run, aPtr.get());
-   return aPtr;
-}
diff --git a/g2log/src/active.hpp b/g2log/src/active.hpp
index e4525c7..ca53e52 100644
--- a/g2log/src/active.hpp
+++ b/g2log/src/active.hpp
@@ -11,7 +11,7 @@
  * and inspired from Herb Sutter's C++11 Active Object
  * http://herbsutter.com/2010/07/12/effective-concurrency-prefer-using-active-objects-instead-of-naked-threads
  *
- * Last update 2013-09-25 by Kjell Hedstrom,
+ * Last update 2013-12-19 by Kjell Hedstrom,
  * e-mail: hedstrom at kjellkod dot cc
  * linkedin: http://linkedin.com/se/kjellkod */
 
@@ -28,21 +28,39 @@ typedef std::function<void() > Callback;
 
 class Active {
 private:
-   Active(); // Construction ONLY through factory createActive();
-   void run();
+   Active() : done_(false) {} // Construction ONLY through factory createActive();
+   Active(const Active&) = delete;
+   Active& operator=(const Active&) = delete;
+
+   void run() {
+      while (!done_) {
+         Callback func;
+         mq_.wait_and_pop(func);
+         func();
+      }
+   }
 
    shared_queue<Callback> mq_;
    std::thread thd_;
-   bool done_; // finished flag : set by ~Active
+   bool done_; 
 
 
 public:
-   virtual ~Active();
-   void send(Callback msg_);
-   static std::unique_ptr<Active> createActive();
+   virtual ~Active() {
+      send([this] { done_ = true;});
+      thd_.join();
+   }
 
-   Active(const Active&) = delete;
-   Active& operator=(const Active&) = delete;
+   void send(Callback msg_) { mq_.push(msg_); }
+
+  /// Factory: safe construction of object before thread start
+   static std::unique_ptr<Active> createActive() {
+      std::unique_ptr<Active> aPtr(new Active());
+      aPtr->thd_ = std::thread(&Active::run, aPtr.get());
+      return aPtr;
+   }
 };
 
+   
+
 } // kjellkod
diff --git a/g2log/src/shared_queue.hpp b/g2log/src/shared_queue.hpp
index 1a8b223..d978474 100644
--- a/g2log/src/shared_queue.hpp
+++ b/g2log/src/shared_queue.hpp
@@ -12,8 +12,7 @@
 * This exampel  was totally inspired by Anthony Williams lock-based data structures in
 * Ref: "C++ Concurrency In Action" http://www.manning.com/williams */
 
-#ifndef SHARED_QUEUE
-#define SHARED_QUEUE
+#pragma once
 
 #include <queue>
 #include <mutex>
@@ -29,15 +28,15 @@ class shared_queue
   mutable std::mutex m_;
   std::condition_variable data_cond_;
 
-  shared_queue& operator=(const shared_queue&); // c++11 feature not yet in vs2010 = delete;
-  shared_queue(const shared_queue& other); // c++11 feature not yet in vs2010 = delete;
+  shared_queue& operator=(const shared_queue&) = delete;
+  shared_queue(const shared_queue& other) = delete;
 
 public:
   shared_queue(){}
 
   void push(T item){
     std::lock_guard<std::mutex> lock(m_);
-    queue_.push(item);
+    queue_.push(std::move(item));
     data_cond_.notify_one();
   }
 
@@ -54,10 +53,12 @@ public:
 
   /// Try to retrieve, if no items, wait till an item is available and try again
   void wait_and_pop(T& popped_item){
-    std::unique_lock<std::mutex> lock(m_); // note: unique_lock is needed for std::condition_variable::wait
+    std::unique_lock<std::mutex> lock(m_); 
     while(queue_.empty())
-    { //                       The 'while' loop below is equal to
-      data_cond_.wait(lock);  //data_cond_.wait(lock, [](bool result){return !queue_.empty();});
+    { 
+       data_cond_.wait(lock);
+       //  This 'while' loop is equal to
+       //  data_cond_.wait(lock, [](bool result){return !queue_.empty();});
     }
     popped_item=std::move(queue_.front());
     queue_.pop();
@@ -73,5 +74,3 @@ public:
     return queue_.size();
   }
 };
-
-#endif