1
\$\begingroup\$

This is a next/prev function for use in a circular array. What do you think?

template <typename T, std::size_t N>
struct circular_array
{
  T a_[N];

  template <std::ptrdiff_t I>
  constexpr auto next(T* const p) noexcept
    requires ((1 == I) || (-1 == I))
  {
    if constexpr((N & (N - 1)) && (1 == I))
      return p == &a_[N - 1] ? a_ : p + 1;
    else if constexpr((N & (N - 1)) && (-1 == I))
      return p == a_ ? &a_[N - 1] : p - 1;
    else
      return &a_[(p - a_ + I) & (N - 1)];
  }
};
\$\endgroup\$
4
  • \$\begingroup\$ N is std::size_t template parameter.” Of what? This code is incomplete. \$\endgroup\$
    – indi
    Commented Dec 26, 2021 at 23:28
  • \$\begingroup\$ You see the array definition, don't you? Yikes, I have trouble editing. I''ll edit should it be really necessary. \$\endgroup\$ Commented Dec 27, 2021 at 2:14
  • 1
    \$\begingroup\$ Looks like just a next function; no prev function in sight, so the code is incomplete. Add the missing function, or edit the title & description. \$\endgroup\$
    – AJNeufeld
    Commented Dec 27, 2021 at 6:27
  • \$\begingroup\$ What about next<-1>()? \$\endgroup\$ Commented Dec 27, 2021 at 9:37

1 Answer 1

1
\$\begingroup\$

Split it into separate prev() and next() functions

Is there a reason why you made next() a template? If you would only write next<1>(...) and next<-1>(...) in your code, then just provide non-templated next() and prev() functions. This simplifies the code as well.

If you do want to keep a templated next(), consider giving I a default value of 1.

Consider providing an iterator-like interface instead

One issue with your code is that the caller needs to keep a raw pointer into the circular array. Instead, I would try to provide an iterator, similar to those provided by the STL containers. A simple one could be:

template <typename T, std::size_t N>
class circular_array
{
  T a_[N];

public:
  class forward_iterator {
    circular_array& array;
    std::size_t i;

  public:
    forward_iterator(circular_array& array): array(array) {}

    T& operator*() {
      return array.a_[i];
    }

    forward_iterator& operator++() {
      ++i;
      if (i == N)
        i = 0;
      return *this;
    }
  };

  forward_iterator begin() {
    return forward_iterator(*this);
  }
};

You can expand this with a reverse iterator and/or make it a bidirectional iterator with both operator++() and operator--() overloads, and you could even consider adding an end iterator that can never be reached. By providing enough of the standard iterator interface, it can then be used like so:

circular_array<int, 10> array = {...};

for (auto value: array | std::ranges::view::take(20)) {
    std::cout << value << "\n";
}

Consider making it a view instead

Going even further, instead of starting with a circular array, you might want to have a regular array, vector or other container, fill it using the standard algorithms, and then loop over it in a circular fashion. So consider making a view that circularly loops over a given container. That way, you could write:

std::vector<int> array = {...};

for (auto value: array | circular_view() | std::ranges::view::take(20)) {
    std::cout << value << "\n";
}
\$\endgroup\$
9
  • \$\begingroup\$ Hey G. Merry and happy everything! I thought a single function template instead of 2 functions would count as positive as this reduces complexity and code copy/pasting IMO and the same approach is followed in the STL (you can supply -1 to std::next()). Mostly, I was worried if the next/prev approach is the fastest one available or maybe is even silly, as I see & (N - 1) applied mostly to indices. \$\endgroup\$ Commented Dec 27, 2021 at 11:01
  • \$\begingroup\$ There is a conversion pointer -> index -> pointer, maybe I should avoid that? \$\endgroup\$ Commented Dec 27, 2021 at 11:13
  • \$\begingroup\$ In std::next(), it's not a template parameter, but a regular parameter, and you can even use it to advance more than one step. But if you do want to follow the STL, consider that there's also std::prev(). I would not worry about the pointer->index->pointer conversion, the compiler should be able to optimize it all away, see for example this result on godbolt.org. \$\endgroup\$
    – G. Sliepen
    Commented Dec 27, 2021 at 12:06
  • \$\begingroup\$ that's true in this particular case, but in general, the conversion might need MUL/DIV. \$\endgroup\$ Commented Dec 27, 2021 at 12:40
  • 1
    \$\begingroup\$ there, I managed to get gcc and clang to generate an IMUL, maybe the times of the pow2 optimization are over. Even if I used indices, this IMUL would be generated IMO. \$\endgroup\$ Commented Dec 27, 2021 at 13:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.