Inquiry about the expected behavior of the comparison operators in RingBuffer
cheekyshibe opened this issue · 1 comments
template <typename T, typename Container, typename Allocator>
inline bool operator==(const ring_buffer<T, Container, Allocator>& a, const ring_buffer<T, Container, Allocator>& b)
{
return (a.size() == b.size()) && (a.c == b.c);
}I have been using your RingBuffer library and recently came across the implementation of the comparison operators, specifically the operator== and operator<=>. However, I noticed that these operators rely on the behavior of the underlying container's comparison operations, rather than considering the circular nature of the RingBuffer.
Based on my understanding, the comparison operators should take into account the circular behavior of the RingBuffer, as the order and position of elements are crucial for accurate comparison. I believe that the current implementation does not reflect this behavior accurately.
To address this concern, I have made some modifications to the code, which involve comparing the elements of the RingBuffer using iterators and considering the circular characteristics. Here are the modified implementations:
template <typename T, typename Container, typename Allocator>
inline bool operator==(const ring_buffer<T, Container, Allocator>& a, const ring_buffer<T, Container, Allocator>& b) {
return eastl::equal(a.begin(), a.end(), b.begin(), b.end());
}I would like to kindly request your input regarding the expected behavior of the comparison operators in the RingBuffer library. Could you please clarify the intended behavior for these operators, specifically considering the circular nature of the RingBuffer?
Test Case:
ring_buffer<int> rb(3);
rb.assign({0, 1, 2, 3, 4});
EXPECT_EQ(rb.size(), 3);
EXPECT_EQ(rb.capacity(), 3);
for (int i = 0; i < 3; ++i) {
EXPECT_EQ(rb[i], i + 2);
}
ring_buffer<int> rb2({3, 4, 2});
EXPECT_EQ(rb.size(), 3);
EXPECT_EQ(rb.capacity(), 3);
ring_buffer<int> rb3({2, 3, 4});
EXPECT_EQ(rb.size(), 3);
EXPECT_EQ(rb.capacity(), 3);
EXPECT_TRUE(rb == rb2); // confused
EXPECT_FALSE(rb == rb2); // expected
EXPECT_TRUE(rb == rb3); // expected If we just compare the underlying container. In rb.c it will be equal with Vector<int> v{3, 4, 2}. So rb == rb2 will get true. After traversing or inserting operations, the subsequent containers will not work, which does not meet my expectations.
I'd be happy to file a pull request for this if we have a consensus.
Hello,
Thanks for the issue report. This looks like a (long standing) bug to me, I agree that conceptually 2 ring buffers should compare equal if they hold the same elements in the same ring-buffer-order, regardless of actual order in the backing container. I'm surprised this has gone so long without being found.
Your proposed implementation looks good and we'd definitely welcome a PR with a fix. If you haven't already, please fill out the CLA form prior to creating the PR.
Cheers.