From 9ff664693531d0b0bf998f9a4952a009da13cd2c Mon Sep 17 00:00:00 2001
From: Rosen Penev <rosenp@gmail.com>
Date: Sat, 1 Jul 2023 07:35:29 -0700
Subject: [PATCH] simplify slice some more

---
 include/exiv2/slice.hpp  | 39 +++++++++++++--------------------------
 unitTests/test_slice.cpp | 28 +++++++++++-----------------
 2 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/include/exiv2/slice.hpp b/include/exiv2/slice.hpp
index e4e23d868f..a05945d093 100644
--- a/include/exiv2/slice.hpp
+++ b/include/exiv2/slice.hpp
@@ -143,7 +143,7 @@ struct ConstSliceBase : SliceBase {
     if (new_end > this->end_) {
       throw std::out_of_range("Invalid input parameters to slice");
     }
-    return slice_type(storage_.data_, new_begin, new_end);
+    return {storage_.data_, new_begin, new_end};
   }
 
  protected:
@@ -176,21 +176,21 @@ struct MutableSliceBase : public ConstSliceBase<storage_type, data_type> {
     return this->storage_.unsafeAt(this->begin_ + index);
   }
 
-  const value_type& at(size_t index) const {
+  [[nodiscard]] const value_type& at(size_t index) const {
     return base_type::at(index);
   }
 
   /*!
    * Obtain an iterator to the first element in the slice.
    */
-  iterator begin() noexcept {
+  [[nodiscard]] iterator begin() noexcept {
     return this->storage_.unsafeGetIteratorAt(this->begin_);
   }
 
   /*!
    * Obtain an iterator to the first element beyond the slice.
    */
-  iterator end() noexcept {
+  [[nodiscard]] iterator end() noexcept {
     return this->storage_.unsafeGetIteratorAt(this->end_);
   }
 
@@ -227,7 +227,7 @@ struct MutableSliceBase : public ConstSliceBase<storage_type, data_type> {
    * mutable_slice_base.
    */
   template <typename slice_type>
-  slice_type subSlice(size_t begin, size_t end) {
+  [[nodiscard]] slice_type subSlice(size_t begin, size_t end) {
     this->rangeCheck(begin);
     // end == size() is a legal value, since end is the first
     // element beyond the slice
@@ -241,7 +241,7 @@ struct MutableSliceBase : public ConstSliceBase<storage_type, data_type> {
     if (new_end > this->end_) {
       throw std::out_of_range("Invalid input parameters to slice");
     }
-    return slice_type(this->storage_.data_, new_begin, new_end);
+    return {this->storage_.data_, new_begin, new_end};
   }
 };
 
@@ -429,24 +429,11 @@ struct Slice : public Internal::MutableSliceBase<Internal::ContainerStorage, con
   using value_type = typename std::remove_cv<typename container::value_type>::type;
 #endif
 
-  /*!
-   * Construct a sub-slice of this slice with the given bounds. The bounds
-   * are evaluated with respect to the current slice.
-   *
-   * @param[in] begin  First element in the new slice.
-   * @param[in] end  First element beyond the new slice.
-   *
-   * @throw std::out_of_range when begin or end are invalid
-   */
-  Slice subSlice(size_t begin, size_t end) {
-    return Internal::MutableSliceBase<Internal::ContainerStorage, container>::template subSlice<Slice>(begin, end);
-  }
-
   /*!
    * Constructs a new constant subSlice. Behaves otherwise exactly like
    * the non-const version.
    */
-  [[nodiscard]] Slice<const container> subSlice(size_t begin, size_t end) const {
+  Slice<const container> subSlice(size_t begin, size_t end) const {
     return this->to_const_base().template subSlice<Slice<const container>>(begin, end);
   }
 };
@@ -530,7 +517,7 @@ struct Slice<T*> : public Internal::MutableSliceBase<Internal::PtrSliceStorage,
  * parameter deduction.
  */
 template <typename T>
-Slice<T> makeSlice(T& cont, size_t begin, size_t end) {
+[[nodiscard]] Slice<T> makeSlice(T& cont, size_t begin, size_t end) {
   return {cont, begin, end};
 }
 
@@ -538,7 +525,7 @@ Slice<T> makeSlice(T& cont, size_t begin, size_t end) {
  * Overload of makeSlice for slices of C-arrays.
  */
 template <typename T>
-Slice<T*> makeSlice(T* ptr, size_t begin, size_t end) {
+[[nodiscard]] Slice<T*> makeSlice(T* ptr, size_t begin, size_t end) {
   return {ptr, begin, end};
 }
 
@@ -546,7 +533,7 @@ Slice<T*> makeSlice(T* ptr, size_t begin, size_t end) {
  * @brief Return a new slice spanning the whole container.
  */
 template <typename container>
-Slice<container> makeSlice(container& cont) {
+[[nodiscard]] Slice<container> makeSlice(container& cont) {
   return {cont, 0, cont.size()};
 }
 
@@ -555,7 +542,7 @@ Slice<container> makeSlice(container& cont) {
  * container.
  */
 template <typename container>
-Slice<container> makeSliceFrom(container& cont, size_t begin) {
+[[nodiscard]] Slice<container> makeSliceFrom(container& cont, size_t begin) {
   return {cont, begin, cont.size()};
 }
 
@@ -563,7 +550,7 @@ Slice<container> makeSliceFrom(container& cont, size_t begin) {
  * @brief Return a new slice spanning until `end`.
  */
 template <typename container>
-Slice<container> makeSliceUntil(container& cont, size_t end) {
+[[nodiscard]] Slice<container> makeSliceUntil(container& cont, size_t end) {
   return {cont, 0, end};
 }
 
@@ -571,7 +558,7 @@ Slice<container> makeSliceUntil(container& cont, size_t end) {
  * Overload of makeSliceUntil for pointer based slices.
  */
 template <typename T>
-Slice<T*> makeSliceUntil(T* ptr, size_t end) {
+[[nodiscard]] Slice<T*> makeSliceUntil(T* ptr, size_t end) {
   return {ptr, 0, end};
 }
 
diff --git a/unitTests/test_slice.cpp b/unitTests/test_slice.cpp
index a730cfe4c5..710bfdb427 100644
--- a/unitTests/test_slice.cpp
+++ b/unitTests/test_slice.cpp
@@ -97,12 +97,10 @@ TYPED_TEST_P(slice, constructionFailsWithZeroLength) {
  * Test the construction of subSlices and their behavior.
  */
 TYPED_TEST_P(slice, subSliceSuccessfulConstruction) {
-  using slice_t = Slice<TypeParam>;
-
   // 0 1 2 3 4 5 6 7 8 9
   //       |     |       center_vals
   //         | |         middle
-  slice_t center_vals = this->getTestSlice(3, 7);
+  auto center_vals = this->getTestSlice(3, 7);
   ASSERT_EQ(center_vals.size(), static_cast<size_t>(4));
   ASSERT_NO_THROW(center_vals.subSlice(1, 3));
 
@@ -110,7 +108,7 @@ TYPED_TEST_P(slice, subSliceSuccessfulConstruction) {
 }
 
 TYPED_TEST_P(slice, subSliceFunctions) {
-  Slice<TypeParam> middle = this->getTestSlice(3, 7).subSlice(1, 3);
+  auto middle = this->getTestSlice(3, 7).subSlice(1, 3);
 
   ASSERT_EQ(middle.size(), static_cast<size_t>(2));
   ASSERT_EQ(middle.at(1), static_cast<typename Slice<TypeParam>::value_type>(5));
@@ -163,11 +161,9 @@ void checkSubSlice(const Slice<T>& sl) {
  * Test that all slices can be also passed as const references and still work
  */
 TYPED_TEST_P(slice, constMethodsPreserveConst) {
-  using slice_t = Slice<TypeParam>;
-
   // 0 1 2 3 4 5 6 7 8 9
   //       |     |       center_vals
-  slice_t center_vals = this->getTestSlice(3, 7);
+  auto center_vals = this->getTestSlice(3, 7);
 
   // check at() const works
   checkConstSliceValueAt(center_vals, 4, 1);
@@ -181,30 +177,28 @@ TYPED_TEST_P(slice, constMethodsPreserveConst) {
  * Test the non-const iterators
  */
 TYPED_TEST_P(mutableSlice, iterators) {
-  using slice_t = Slice<TypeParam>;
-  slice_t sl = this->getTestSlice();
+  auto sl = this->getTestSlice();
 
-  ASSERT_EQ(*sl.begin(), static_cast<typename slice_t::value_type>(1));
-  ASSERT_EQ(*sl.end(), static_cast<typename slice_t::value_type>(this->vec_size - 1));
+  ASSERT_EQ(*sl.begin(), static_cast<typename decltype(sl)::value_type>(1));
+  ASSERT_EQ(*sl.end(), static_cast<typename decltype(sl)::value_type>(this->vec_size - 1));
 
   for (auto it = sl.begin(); it < sl.end(); ++it) {
-    *it = 2 * (*it);
+    *it *= 2;
   }
 
   ASSERT_EQ(this->vec_.at(0), 0);
   for (size_t j = 1; j < this->vec_size - 1; ++j) {
-    ASSERT_EQ(this->vec_.at(j), static_cast<typename slice_t::value_type>(2 * j));
+    ASSERT_EQ(this->vec_.at(j), static_cast<typename decltype(sl)::value_type>(2 * j));
     ASSERT_EQ(this->vec_.at(j), sl.at(j - 1));
   }
-  ASSERT_EQ(this->vec_.at(this->vec_size - 1), static_cast<typename slice_t::value_type>(this->vec_size - 1));
+  ASSERT_EQ(this->vec_.at(this->vec_size - 1), static_cast<typename decltype(sl)::value_type>(this->vec_size - 1));
 }
 
 /*!
  * Test the non-const version of at()
  */
 TYPED_TEST_P(mutableSlice, at) {
-  using slice_t = Slice<TypeParam>;
-  slice_t sl = this->getTestSlice(2, 4);
+  auto sl = this->getTestSlice(2, 4);
 
   sl.at(0) = 6;
   sl.at(1) = 12;
@@ -215,7 +209,7 @@ TYPED_TEST_P(mutableSlice, at) {
     if (j == 2 || j == 3) {
       continue;
     }
-    ASSERT_EQ(this->vec_.at(j), static_cast<typename slice_t::value_type>(j));
+    ASSERT_EQ(this->vec_.at(j), static_cast<typename decltype(sl)::value_type>(j));
   }
 }