From a08b17fa8a156a8ee3768be7271c0d91a37c6990 Mon Sep 17 00:00:00 2001
From: ExMix <rahuba.youri@mapswithme.com>
Date: Wed, 30 Oct 2013 17:05:44 +0300
Subject: [PATCH] review fixes

---
 base/base_tests/thread_pool_tests.cpp | 2 ++
 base/thread.hpp                       | 6 ++----
 base/thread_pool.cpp                  | 8 ++++----
 base/thread_pool.hpp                  | 3 ++-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/base/base_tests/thread_pool_tests.cpp b/base/base_tests/thread_pool_tests.cpp
index d42e3b00ea..fa9b6f34ad 100644
--- a/base/base_tests/thread_pool_tests.cpp
+++ b/base/base_tests/thread_pool_tests.cpp
@@ -87,6 +87,7 @@ UNIT_TEST(ThreadPool_ExecutionTaskTest)
   vector<threads::IRoutine *> tasks;
   for (int i = 0; i < TASK_COUNT - 1; ++i)
     tasks.push_back(new CancelTestTask(true));
+  // CancelTastTask::Do method should not be called for last task
   tasks.push_back(new CancelTestTask(false));
 
   int finishCounter = 0;
@@ -96,6 +97,7 @@ UNIT_TEST(ThreadPool_ExecutionTaskTest)
   for (size_t i = 0; i < tasks.size(); ++i)
     pool.AddTask(tasks[i]);
 
+  // CancelTastTask::Do method should not be called for last task
   tasks.back()->Cancel();
   tasks.clear();
 
diff --git a/base/thread.hpp b/base/thread.hpp
index f3b26a5cbb..8c272ebe1c 100644
--- a/base/thread.hpp
+++ b/base/thread.hpp
@@ -5,6 +5,7 @@
 #include "../std/stdint.hpp"
 #include "../std/vector.hpp"
 #include "../std/utility.hpp"
+#include "../std/noncopyable.hpp"
 
 #ifdef OMIM_OS_WINDOWS
 #include "../std/windows.hpp" // for DWORD
@@ -54,14 +55,11 @@ namespace threads
   };
 
   /// Simple threads container. Takes ownership for every added IRoutine.
-  class SimpleThreadPool
+  class SimpleThreadPool : public noncopyable
   {
     typedef pair<Thread *, IRoutine *> ValueT;
     vector<ValueT> m_pool;
 
-    SimpleThreadPool(SimpleThreadPool const &);
-    SimpleThreadPool & operator=(Thread const &);
-
   public:
     SimpleThreadPool(size_t reserve = 0);
     ~SimpleThreadPool();
diff --git a/base/thread_pool.cpp b/base/thread_pool.cpp
index 4071d6d3c4..f16f30b205 100644
--- a/base/thread_pool.cpp
+++ b/base/thread_pool.cpp
@@ -15,7 +15,7 @@ namespace threads
     class PoolRoutine : public IRoutine
     {
     public:
-      PoolRoutine(pop_routine_fn popFn, finish_routine_fn finishFn)
+      PoolRoutine(const pop_routine_fn & popFn, const finish_routine_fn & finishFn)
         : m_popFn(popFn)
         , m_finishFn(finishFn)
       {
@@ -47,9 +47,9 @@ namespace threads
   class ThreadPool::Impl
   {
   public:
-    Impl(size_t size, finish_routine_fn finishFn)
+    Impl(size_t size, const finish_routine_fn & finishFn)
     {
-      m_threads.reserve(size);
+      m_threads.resize(size);
       for (size_t i = 0; i < size; ++i)
       {
         thread_info_t info = make_pair(new threads::Thread(), new PoolRoutine(bind(&ThreadPool::Impl::PopFront, this),
@@ -97,7 +97,7 @@ namespace threads
     vector<thread_info_t> m_threads;
   };
 
-  ThreadPool::ThreadPool(size_t size, finish_routine_fn finishFn)
+  ThreadPool::ThreadPool(size_t size, const finish_routine_fn & finishFn)
     : m_impl(new Impl(size, finishFn)) {}
 
   void ThreadPool::AddTask(threads::IRoutine * routine)
diff --git a/base/thread_pool.hpp b/base/thread_pool.hpp
index db83492735..2bdf4a0706 100644
--- a/base/thread_pool.hpp
+++ b/base/thread_pool.hpp
@@ -11,8 +11,9 @@ namespace threads
   class ThreadPool
   {
   public:
-    ThreadPool(size_t size, finish_routine_fn finishFn);
+    ThreadPool(size_t size, const finish_routine_fn & finishFn);
 
+    // ThreadPool will not delete routine. You can delete it in finish_routine_fn if need
     void AddTask(threads::IRoutine * routine);
     void Stop();