Merge pull request #28 from lycantropos/smart-pointers

Simplifying API

cauldron/bases.h

@@ -2,6 +2,7 @@
 
 #include <memory>
 #include <random>
+
 #include "sieve.h"
 #include "facility.h"
 
@@ -40,8 +41,7 @@ class Strategy {
    * that generates values from either original strategy or provided one.
    */
   virtual Union<Value> operator||(const Strategy<Value> &strategy) const {
-    return Union<Value>{std::move(clone()),
-                        std::move(strategy.clone())};
+    return Union<Value>{*this, strategy};
   }
 
   /**
@@ -56,30 +56,24 @@ class Strategy {
   /**
    * Returns a new strategy
    * that generates values from the strategy
-   * modified with provided ``strategies::Converter`` instance.
+   * which satisfy provided ``strategies::Requirement`` instance.
+   *
+   * Note that if the ``requirement`` is too hard to satisfy
+   * this might result in failing with ``OutOfCycles``.
    */
-  virtual std::unique_ptr<Mapped<Value>> map(
-      const Converter<Value> &converter
-  ) const {
-    Facility<Value> facility{converter};
-    return std::make_unique<Mapped<Value>>(facility,
-                                           std::move(clone()));
+  virtual Filtered<Value> filter(const Requirement<Value> &requirement) const {
+    Sieve<Value> sieve{requirement};
+    return Filtered<Value>(sieve, *this);
   }
 
   /**
    * Returns a new strategy
    * that generates values from the strategy
-   * which satisfy provided ``strategies::Requirement`` instance.
-   *
-   * Note that if the ``requirement`` is too hard to satisfy
-   * this might result in failing with ``OutOfCycles``.
+   * modified with provided ``strategies::Converter`` instance.
    */
-  virtual std::unique_ptr<Filtered<Value>> filter(
-      const Requirement<Value> &requirement
-  ) const {
-    Sieve<Value> sieve{requirement};
-    return std::make_unique<Filtered<Value>>(sieve,
-                                             std::move(clone()));
+  virtual Mapped<Value> map(const Converter<Value> &converter) const {
+    Facility<Value> facility{converter};
+    return Mapped<Value>(facility, *this);
   }
 
   /**
@@ -117,18 +111,22 @@ class CloneHelper : public Strategy<Value> {
 template<class Value>
 class Union : public CloneHelper<Value, Union<Value>> {
  public:
-  Union(std::initializer_list<std::shared_ptr<Strategy<Value>>> strategies) {
-    if (strategies.size() == 0) {
-      throw std::invalid_argument("``strategies`` should be non-empty.");
-    }
-    strategies_ = strategies;
+  explicit Union(const Strategy<Value> &strategy,
+                 const Strategy<Value> &other_strategy) {
+    strategies_.push_back(strategy.clone());
+    strategies_.push_back(other_strategy.clone());
   }
 
-  explicit Union(std::vector<std::shared_ptr<Strategy<Value>>> strategies) {
-    if (strategies.size() == 0) {
-      throw std::invalid_argument("``strategies`` should be non-empty.");
+  /**
+   * Default copy constructor doesn't fit
+   * since we're using ``std::unique_ptr`` in class member
+   * which is not copyable.
+   */
+  Union(const Union<Value> &strategy) {
+    strategies_.reserve(strategy.strategies_.size());
+    for (const auto &sub_strategy: strategy.strategies_) {
+      strategies_.push_back(sub_strategy->clone());
     }
-    strategies_ = std::move(strategies);
   }
 
   /**
@@ -143,21 +141,21 @@ class Union : public CloneHelper<Value, Union<Value>> {
   }
 
   Union<Value> operator||(const Strategy<Value> &strategy) const override {
-    std::vector<std::shared_ptr<Strategy<Value>>> strategies(strategies_);
-    strategies.push_back(std::move(strategy.clone()));
-    return Union<Value>(strategies);
+    Union<Value> result(*this);
+    result.strategies_.push_back(strategy.clone());
+    return result;
   }
 
   Union<Value> operator||(const Union<Value> &strategy) const override {
-    std::vector<std::shared_ptr<Strategy<Value>>> strategies(strategies_);
-    strategies.insert(strategies.begin(),
-                      strategy.strategies_.begin(),
-                      strategy.strategies_.end());
-    return Union<Value>(strategies);
+    Union<Value> result(*this);
+    for (const auto &sub_strategy: strategy.strategies_) {
+      result.strategies_.push_back(sub_strategy->clone());
+    }
+    return result;
   }
 
  private:
-  std::vector<std::shared_ptr<Strategy<Value>>> strategies_;
+  std::vector<std::unique_ptr<Strategy<Value>>> strategies_;
 };
 
 
@@ -175,16 +173,24 @@ template<typename Value>
 class Filtered : public CloneHelper<Value, Filtered<Value>> {
  public:
   explicit Filtered(const Sieve<Value> &sieve,
-                    std::shared_ptr<Strategy<Value>> strategy) :
+                    const Strategy<Value> &strategy) :
       sieve_(sieve),
-      strategy_(std::move(strategy)) {};
+      strategy_(strategy.clone()) {};
 
-  std::unique_ptr<Filtered<Value>> filter(
+  /**
+   * Default copy constructor doesn't fit
+   * since we're using ``std::unique_ptr`` as class member
+   * which is not copyable.
+   */
+  Filtered(const Filtered<Value> &strategy) :
+      sieve_(strategy.sieve_),
+      strategy_(strategy.strategy_->clone()) {}
+
+  Filtered<Value> filter(
       const Requirement<Value> &requirement
   ) const override {
     auto sieve = sieve_.expand(requirement);
-    return std::make_unique<Filtered<Value>>(sieve,
-                                             strategy_);
+    return Filtered<Value>(sieve, *strategy_);
   }
 
   /**
@@ -201,7 +207,7 @@ class Filtered : public CloneHelper<Value, Filtered<Value>> {
 
  protected:
   Sieve<Value> sieve_;
-  std::shared_ptr<Strategy<Value>> strategy_;
+  std::unique_ptr<Strategy<Value>> strategy_;
 };
 
 
@@ -219,16 +225,22 @@ template<typename Value>
 class Mapped : public CloneHelper<Value, Mapped<Value>> {
  public:
   explicit Mapped(const Facility<Value> &facility,
-                  std::shared_ptr<Strategy<Value>> strategy) :
+                  const Strategy<Value> &strategy) :
       facility_(facility),
-      strategy_(std::move(strategy)) {};
+      strategy_(strategy.clone()) {};
 
-  std::unique_ptr<Mapped<Value>> map(
-      const Converter<Value> &converter
-  ) const override {
+  /**
+   * Default copy constructor doesn't fit
+   * since we're using ``std::unique_ptr`` as class member
+   * which is not copyable.
+   */
+  Mapped(const Mapped<Value> &strategy) :
+      facility_(strategy.facility_),
+      strategy_(strategy.strategy_->clone()) {}
+
+  Mapped<Value> map(const Converter<Value> &converter) const override {
     auto facility = facility_.expand(converter);
-    return std::make_unique<Mapped>(facility,
-                                    strategy_);
+    return Mapped(facility, *strategy_);
   }
 
   /**
@@ -242,6 +254,6 @@ class Mapped : public CloneHelper<Value, Mapped<Value>> {
 
  protected:
   Facility<Value> facility_;
-  std::shared_ptr<Strategy<Value>> strategy_;
+  std::unique_ptr<Strategy<Value>> strategy_;
 };
 }

cauldron/builder.h

@@ -17,8 +17,19 @@ namespace cauldron {
 template<class Object, class ...Value>
 class Builder : public CloneHelper<Object, Builder<Object, Value...>> {
  public:
-  explicit Builder(std::shared_ptr<cauldron::Strategy<Value>>... strategy) :
-      strategies_(std::make_tuple(strategy...)) {}
+  /**
+   * @param strategy: strategy to generate constructor arguments from.
+   */
+  explicit Builder(const cauldron::Strategy<Value> &... strategy) :
+      strategies_(std::make_tuple(strategy.clone()...)) {}
+
+  /**
+   * Default copy constructor doesn't fit
+   * since we're using ``std::unique_ptr`` as class members
+   * which is not copyable.
+   */
+  Builder(const Builder<Object, Value...> &builder) :
+      strategies_(builder.clone_strategies()) {}
 
   /**
    * Generates pseudo-random ``Object`` instance.
@@ -30,28 +41,58 @@ class Builder : public CloneHelper<Object, Builder<Object, Value...>> {
   }
 
  private:
-  std::tuple<std::shared_ptr<cauldron::Strategy<Value>>...> strategies_;
+  std::tuple<std::unique_ptr<cauldron::Strategy<Value>>...> strategies_;
 
   /**
    * Helper function for unpacking ``Builder::strategies_`` tuple
    * into variadic ``Strategy`` instances.
    */
   template<std::size_t... Indices>
   Object produce(
-      std::tuple<std::shared_ptr<cauldron::Strategy<Value>>...> tuple,
+      const std::tuple<std::unique_ptr<cauldron::Strategy<Value>>...> &strategies,
       std::index_sequence<Indices...>
   ) const {
-    return produce(std::get<Indices>(tuple)...);
+    return produce(std::get<Indices>(strategies)...);
   }
 
   /**
    * Helper function for producing values
    * from variadic ``Strategy`` instances.
    */
   Object produce(
-      std::shared_ptr<cauldron::Strategy<Value>>... strategy
+      const std::unique_ptr<cauldron::Strategy<Value>> &... strategy
+  ) const {
+    return Object((*strategy)()...);
+  }
+
+  std::tuple<std::unique_ptr<cauldron::Strategy<Value>>...>
+  clone_strategies() const {
+    auto strategies_count = std::tuple_size<decltype(strategies_)>{};
+    return clone_strategies(strategies_,
+                            std::make_index_sequence<strategies_count>{});
+  }
+
+  /**
+   * Helper function for unpacking ``Builder::strategies_`` tuple
+   * into variadic ``Strategy`` instances.
+   */
+  template<std::size_t... Indices>
+  std::tuple<std::unique_ptr<cauldron::Strategy<Value>>...>
+  clone_strategies(
+      const std::tuple<std::unique_ptr<cauldron::Strategy<Value>>...> &strategies,
+      std::index_sequence<Indices...>
+  ) const {
+    return clone_strategies(std::get<Indices>(strategies)...);
+  }
+
+  /**
+   * Helper function for cloning variadic ``Strategy`` instances.
+   */
+  std::tuple<std::unique_ptr<cauldron::Strategy<Value>>...>
+  clone_strategies(
+      const std::unique_ptr<cauldron::Strategy<Value>> &... strategy
   ) const {
-    return Object(strategy->operator()()...);
+    return std::make_tuple(strategy->clone()...);
   }
 };
 }

cauldron/floats.h

@@ -4,6 +4,7 @@
 #include <algorithm>
 #include <limits>
 #include <functional>
+
 #include "bases.h"
 
 

cauldron/integers.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <limits>
+
 #include "bases.h"
 
 

cauldron/sets.h

@@ -1,6 +1,8 @@
 #pragma once
 
+#include <algorithm>
 #include <set>
+
 #include "bases.h"
 
 
@@ -19,10 +21,19 @@ class Sets : public CloneHelper<std::set<Element>, Sets<Element>> {
    * @param sizes: strategy to generate sets sizes from.
    * @param elements: strategy to generate sets elements from.
    */
-  Sets(std::shared_ptr<SizesStrategy> sizes,
-       std::shared_ptr<ElementsStrategy> elements) :
-      sizes_(std::move(sizes)),
-      elements_(std::move(elements)) {};
+  Sets(const SizesStrategy &sizes,
+       const ElementsStrategy &elements) :
+      sizes_(sizes.clone()),
+      elements_(elements.clone()) {};
+
+  /**
+   * Default copy constructor doesn't fit
+   * since we're using ``std::unique_ptr`` as class members
+   * which is not copyable.
+   */
+  Sets(const Sets<Element> &sets) :
+      sizes_(sets.sizes_->clone()),
+      elements_(sets.elements_->clone()) {};
 
   /**
    * Generates pseudo-random ``std::set`` instance.
@@ -34,20 +45,16 @@ class Sets : public CloneHelper<std::set<Element>, Sets<Element>> {
         [&result](Element value) -> bool {
           return result.find(value) == result.end();
         });
-    std::shared_ptr<ElementsStrategy> elements(
-        elements_->filter(element_unique));
-    // FIXME: workaround using bind to get producer from strategy
-    auto elements_producer = std::bind(&ElementsStrategy::operator(),
-                                       elements);
+    Filtered<Element> elements = (*elements_).filter(element_unique);
     std::generate_n(std::inserter(result,
                                   result.begin()),
                     size,
-                    elements_producer);
+                    elements);
     return result;
   }
 
  private:
-  std::shared_ptr<SizesStrategy> sizes_;
-  std::shared_ptr<ElementsStrategy> elements_;
+  std::unique_ptr<SizesStrategy> sizes_;
+  std::unique_ptr<ElementsStrategy> elements_;
 };
 }

cauldron/strings.cpp

@@ -1,20 +1,27 @@
 #include <algorithm>
+
 #include "strings.h"
 
 
 namespace cauldron {
-Strings::Strings(std::shared_ptr<Strategy<size_t>> lengths,
-                 std::shared_ptr<Strategy<char>> alphabet) :
-    lengths_(std::move(lengths)),
-    alphabet_(std::move(alphabet)) {}
+Strings::Strings(const Strategy<size_t> &lengths,
+                 const Strategy<char> &alphabet) :
+    lengths_(lengths.clone()),
+    alphabet_(alphabet.clone()) {}
+
+
+Strings::Strings(const Strings &strings) :
+    lengths_(strings.lengths_->clone()),
+    alphabet_(strings.alphabet_->clone()) {}
 
 
 std::string Strings::operator()() const {
   size_t length = (*lengths_)();
   std::string result(length, 0);
-  // FIXME: workaround using bind to get producer from strategy
-  auto characters_producer = std::bind(&Strategy<char>::operator(),
-                                       alphabet_);
+  // FIXME: workaround using lambda to get producer from strategy
+  auto characters_producer = [&]() -> char {
+    return (*alphabet_)();
+  };
   std::generate_n(result.begin(),
                   length,
                   characters_producer);