From fdfcf804a8b43aa4f480480215544d45efb58ea8 Mon Sep 17 00:00:00 2001
From: Hans Dembinski <hans.dembinski@gmail.com>
Date: Sun, 11 Nov 2018 13:50:27 +0100
Subject: [PATCH] refactoring of make_histogram factories

---
 include/boost/histogram.hpp                |  1 +
 include/boost/histogram/histogram.hpp      | 88 ++++++----------------
 include/boost/histogram/make_histogram.hpp | 65 ++++++++++++++++
 test/histogram_dynamic_test.cpp            |  2 +-
 test/histogram_test.cpp                    | 10 +--
 test/utility_histogram.hpp                 |  2 +-
 6 files changed, 94 insertions(+), 74 deletions(-)
 create mode 100644 include/boost/histogram/make_histogram.hpp

diff --git a/include/boost/histogram.hpp b/include/boost/histogram.hpp
index f90e22ec..a3833cda 100644
--- a/include/boost/histogram.hpp
+++ b/include/boost/histogram.hpp
@@ -18,6 +18,7 @@
 #include <boost/histogram/axis/variant.hpp>
 #include <boost/histogram/histogram.hpp>
 #include <boost/histogram/literals.hpp>
+#include <boost/histogram/make_histogram.hpp>
 #include <boost/histogram/storage_adaptor.hpp>
 #include <boost/histogram/weight.hpp>
 
diff --git a/include/boost/histogram/histogram.hpp b/include/boost/histogram/histogram.hpp
index 895eb451..dde153aa 100644
--- a/include/boost/histogram/histogram.hpp
+++ b/include/boost/histogram/histogram.hpp
@@ -1,4 +1,4 @@
-// Copyright 2015-2017 Hans Dembinski
+// Copyright 2015-2018 Hans Dembinski
 //
 // Distributed under the Boost Software License, Version 1.0.
 // (See accompanying file LICENSE_1_0.txt
@@ -9,14 +9,12 @@
 
 #include <algorithm>
 #include <boost/assert.hpp>
-#include <boost/histogram/adaptive_storage.hpp> // implements default_storage
 #include <boost/histogram/arithmetic_operators.hpp>
 #include <boost/histogram/detail/axes.hpp>
 #include <boost/histogram/detail/index_mapper.hpp>
 #include <boost/histogram/detail/meta.hpp>
 #include <boost/histogram/histogram_fwd.hpp>
 #include <boost/histogram/iterator.hpp>
-#include <boost/histogram/storage_adaptor.hpp>
 #include <boost/mp11.hpp>
 #include <functional>
 #include <tuple>
@@ -26,15 +24,13 @@
 namespace boost {
 namespace histogram {
 
-template <typename Axes, typename Container>
+template <typename Axes, typename Storage>
 class histogram {
   static_assert(mp11::mp_size<Axes>::value > 0, "at least one axis required");
 
 public:
   using axes_type = Axes;
-  using container_type = Container;
-  using storage_type =
-      mp11::mp_if<detail::is_storage<Container>, Container, storage_adaptor<Container>>;
+  using storage_type = Storage;
   using value_type = typename storage_type::value_type;
   using const_reference = typename storage_type::const_reference;
   using const_iterator = iterator<histogram>;
@@ -45,40 +41,40 @@ public:
   histogram& operator=(const histogram& rhs) = default;
   histogram& operator=(histogram&& rhs) = default;
 
-  template <typename A, typename C>
-  explicit histogram(const histogram<A, C>& rhs) : storage_(rhs.storage_) {
+  template <typename A, typename S>
+  explicit histogram(const histogram<A, S>& rhs) : storage_(rhs.storage_) {
     detail::axes_assign(axes_, rhs.axes_);
   }
 
-  template <typename A, typename C>
-  histogram& operator=(const histogram<A, C>& rhs) {
+  template <typename A, typename S>
+  histogram& operator=(const histogram<A, S>& rhs) {
     storage_ = rhs.storage_;
     detail::axes_assign(axes_, rhs.axes_);
     return *this;
   }
 
-  explicit histogram(const axes_type& a, container_type c = container_type())
-      : axes_(a), storage_(std::move(c)) {
+  explicit histogram(const axes_type& a, storage_type s = {})
+      : axes_(a), storage_(std::move(s)) {
     storage_.reset(detail::bincount(axes_));
   }
 
-  explicit histogram(axes_type&& a, container_type c = container_type())
-      : axes_(std::move(a)), storage_(std::move(c)) {
+  explicit histogram(axes_type&& a, storage_type s = {})
+      : axes_(std::move(a)), storage_(std::move(s)) {
     storage_.reset(detail::bincount(axes_));
   }
 
-  template <typename A, typename C>
-  bool operator==(const histogram<A, C>& rhs) const noexcept {
+  template <typename A, typename S>
+  bool operator==(const histogram<A, S>& rhs) const noexcept {
     return detail::axes_equal(axes_, rhs.axes_) && storage_ == rhs.storage_;
   }
 
-  template <typename A, typename C>
-  bool operator!=(const histogram<A, C>& rhs) const noexcept {
+  template <typename A, typename S>
+  bool operator!=(const histogram<A, S>& rhs) const noexcept {
     return !operator==(rhs);
   }
 
-  template <typename A, typename C>
-  histogram& operator+=(const histogram<A, C>& rhs) {
+  template <typename A, typename S>
+  histogram& operator+=(const histogram<A, S>& rhs) {
     if (!detail::axes_equal(axes_, rhs.axes_))
       throw std::invalid_argument("axes of histograms differ");
     storage_ += rhs.storage_;
@@ -187,11 +183,10 @@ public:
     using sub_axes_type = detail::sub_axes<axes_type, N, Ns...>;
     using HR = histogram<sub_axes_type, storage_type>;
     auto sub_axes = detail::make_sub_axes(axes_, N(), Ns()...);
-    // make something here to copy allocator if container has an allocator
     auto hr = HR(std::move(sub_axes),
-                 detail::static_if<detail::has_allocator<container_type>>(
-                     [this](auto) { return container_type(storage_.get_allocator()); },
-                     [](auto) { return container_type(); }, 0));
+                 detail::static_if<detail::has_allocator<storage_type>>(
+                     [this](auto) { return storage_type(storage_.get_allocator()); },
+                     [](auto) { return storage_type(); }, 0));
     const auto b = detail::bool_mask<N, Ns...>(rank(), true);
     std::vector<unsigned> shape(rank());
     for_each_axis(detail::shape_collector(shape.begin()));
@@ -240,50 +235,9 @@ private:
   friend class histogram;
   template <typename H>
   friend class iterator;
+  friend class unsafe_access;
 };
 
-/// static type factory with custom storage type
-template <typename S, typename T, typename... Ts, typename = detail::requires_axis<T>>
-auto make_histogram_with(S&& s, T&& axis0, Ts&&... axis) {
-  auto axes = std::make_tuple(std::forward<T>(axis0), std::forward<Ts>(axis)...);
-  return histogram<decltype(axes), detail::unqual<S>>(std::move(axes),
-                                                      std::forward<S>(s));
-}
-
-/// static type factory with standard storage type
-template <typename T, typename... Ts, typename = detail::requires_axis<T>>
-auto make_histogram(T&& axis0, Ts&&... axis) {
-  return make_histogram_with(default_storage(), std::forward<T>(axis0),
-                             std::forward<Ts>(axis)...);
-}
-
-/// dynamic type factory from vector-like with custom storage type
-template <typename S, typename T, typename = detail::requires_axis_vector<T>>
-auto make_histogram_with(S&& s, T&& t) {
-  return histogram<detail::unqual<T>, detail::unqual<S>>(std::forward<T>(t),
-                                                         std::forward<S>(s));
-}
-
-/// dynamic type factory from vector-like with standard storage type
-template <typename T, typename = detail::requires_axis_vector<T>>
-auto make_histogram(T&& t) {
-  return make_histogram_with(default_storage(), std::forward<T>(t));
-}
-
-/// dynamic type factory from iterator range with custom storage type
-template <typename Storage, typename Iterator,
-          typename = detail::requires_iterator<Iterator>>
-auto make_histogram_with(Storage&& s, Iterator begin, Iterator end) {
-  using T = detail::iterator_value_type<Iterator>;
-  auto axes = std::vector<T>(begin, end);
-  return make_histogram_with(std::forward<Storage>(s), std::move(axes));
-}
-
-/// dynamic type factory from iterator range with standard storage type
-template <typename Iterator, typename = detail::requires_iterator<Iterator>>
-auto make_histogram(Iterator begin, Iterator end) {
-  return make_histogram_with(default_storage(), begin, end);
-}
 } // namespace histogram
 } // namespace boost
 
diff --git a/include/boost/histogram/make_histogram.hpp b/include/boost/histogram/make_histogram.hpp
new file mode 100644
index 00000000..a9e9f054
--- /dev/null
+++ b/include/boost/histogram/make_histogram.hpp
@@ -0,0 +1,65 @@
+// Copyright 2015-2018 Hans Dembinski
+//
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt
+// or copy at http://www.boost.org/LICENSE_1_0.txt)
+
+#ifndef BOOST_HISTOGRAM_MAKE_HISTOGRAM_HPP
+#define BOOST_HISTOGRAM_MAKE_HISTOGRAM_HPP
+
+#include <boost/histogram/adaptive_storage.hpp> // implements default_storage
+#include <boost/histogram/histogram.hpp>
+#include <boost/histogram/storage_adaptor.hpp>
+#include <tuple>
+
+namespace boost {
+namespace histogram {
+
+/// histogram factory from compile-time axis configuration with custom storage
+template <typename C, typename T, typename... Ts, typename = detail::requires_axis<T>>
+auto make_histogram_with(C&& c, T&& axis0, Ts&&... axis) {
+  using CU = detail::unqual<C>;
+  using S = mp11::mp_if<detail::is_storage<CU>, CU, storage_adaptor<CU>>;
+  auto axes = std::make_tuple(std::forward<T>(axis0), std::forward<Ts>(axis)...);
+  return histogram<decltype(axes), S>(std::move(axes), std::forward<C>(c));
+}
+
+/// histogram factory from compile-time axis configuration with standard storage
+template <typename T, typename... Ts, typename = detail::requires_axis<T>>
+auto make_histogram(T&& axis0, Ts&&... axis) {
+  return make_histogram_with(default_storage(), std::forward<T>(axis0),
+                             std::forward<Ts>(axis)...);
+}
+
+/// histogram factory from vector-like with custom storage
+template <typename C, typename T, typename = detail::requires_axis_vector<T>>
+auto make_histogram_with(C&& c, T&& t) {
+  using CU = detail::unqual<C>;
+  using S = mp11::mp_if<detail::is_storage<CU>, CU, storage_adaptor<CU>>;
+  return histogram<detail::unqual<T>, S>(std::forward<T>(t), std::forward<C>(c));
+}
+
+/// dynamic type factory from vector-like with default storage
+template <typename T, typename = detail::requires_axis_vector<T>>
+auto make_histogram(T&& t) {
+  return make_histogram_with(default_storage(), std::forward<T>(t));
+}
+
+/// histogram factory from iterator range with custom storage
+template <typename C, typename Iterator, typename = detail::requires_iterator<Iterator>>
+auto make_histogram_with(C&& c, Iterator begin, Iterator end) {
+  using T = detail::iterator_value_type<Iterator>;
+  auto axes = std::vector<T>(begin, end);
+  return make_histogram_with(std::forward<C>(c), std::move(axes));
+}
+
+/// dynamic type factory from iterator range with default storage
+template <typename Iterator, typename = detail::requires_iterator<Iterator>>
+auto make_histogram(Iterator begin, Iterator end) {
+  return make_histogram_with(default_storage(), begin, end);
+}
+
+} // namespace histogram
+} // namespace boost
+
+#endif
diff --git a/test/histogram_dynamic_test.cpp b/test/histogram_dynamic_test.cpp
index 233064c8..f383f817 100644
--- a/test/histogram_dynamic_test.cpp
+++ b/test/histogram_dynamic_test.cpp
@@ -8,8 +8,8 @@
 #include <array>
 #include <boost/core/lightweight_test.hpp>
 #include <boost/histogram/adaptive_storage.hpp>
-#include <boost/histogram/histogram.hpp>
 #include <boost/histogram/literals.hpp>
+#include <boost/histogram/make_histogram.hpp>
 #include <boost/histogram/ostream_operators.hpp>
 #include <boost/histogram/storage_adaptor.hpp>
 #include <cstdlib>
diff --git a/test/histogram_test.cpp b/test/histogram_test.cpp
index df565fca..2474e395 100644
--- a/test/histogram_test.cpp
+++ b/test/histogram_test.cpp
@@ -104,9 +104,9 @@ void run_tests() {
     auto h = make(Tag(), axis::integer<>{0, 2}, axis::integer<>{0, 3});
     h(0, 0);
     auto h2 = decltype(h)(h);
-    BOOST_TEST(h2 == h);
-    auto h3 =
-        histogram<std::tuple<axis::integer<>, axis::integer<>>, std::vector<unsigned>>(h);
+    BOOST_TEST_EQ(h2, h);
+    auto h3 = histogram<std::tuple<axis::integer<>, axis::integer<>>,
+                        storage_adaptor<std::vector<unsigned>>>(h);
     BOOST_TEST_EQ(h3, h);
   }
 
@@ -121,8 +121,8 @@ void run_tests() {
     // test self-assign
     h2 = h2;
     BOOST_TEST_EQ(h, h2);
-    auto h3 =
-        histogram<std::tuple<axis::integer<>, axis::integer<>>, std::vector<unsigned>>();
+    auto h3 = histogram<std::tuple<axis::integer<>, axis::integer<>>,
+                        storage_adaptor<std::vector<unsigned>>>();
     h3 = h;
     BOOST_TEST_EQ(h, h3);
   }
diff --git a/test/utility_histogram.hpp b/test/utility_histogram.hpp
index dde13c6d..78970eb6 100644
--- a/test/utility_histogram.hpp
+++ b/test/utility_histogram.hpp
@@ -8,7 +8,7 @@
 #define BOOST_HISTOGRAM_TEST_UTILITY_HISTOGRAM_HPP
 
 #include <boost/histogram/axis/variant.hpp>
-#include <boost/histogram/histogram.hpp>
+#include <boost/histogram/make_histogram.hpp>
 #include <numeric>
 #include <ostream>
 #include <type_traits>