PragmaTwice commented on code in PR #3268:
URL: https://github.com/apache/kvrocks/pull/3268#discussion_r2548893293
##########
src/types/redis_tdigest.h:
##########
@@ -33,6 +33,126 @@
#include "tdigest.h"
namespace redis {
+
+// TODO: It should be replaced by a iteration of the rocksdb iterator
+class DummyCentroids {
+ public:
+ class BaseIterator {
+ public:
+ virtual ~BaseIterator() = default;
+ virtual bool Next() = 0;
+ virtual bool Prev() = 0;
+ virtual bool Valid() const = 0;
+ virtual std::unique_ptr<BaseIterator> Clone() const = 0;
+ virtual StatusOr<Centroid> GetCentroid() const = 0;
+ };
+
+ DummyCentroids(const TDigestMetadata& meta_data, const
std::vector<Centroid>& centroids)
+ : meta_data_(meta_data), centroids_(centroids) {}
+ class Iterator : public BaseIterator {
+ public:
+ Iterator(std::vector<Centroid>::const_iterator&& iter, const
std::vector<Centroid>& centroids)
+ : iter_(iter), centroids_(centroids) {}
+ std::unique_ptr<BaseIterator> Clone() const override {
+ if (iter_ != centroids_.cend()) {
+ return std::make_unique<Iterator>(std::next(centroids_.cbegin(),
std::distance(centroids_.cbegin(), iter_)),
+ centroids_);
+ }
+ return std::make_unique<Iterator>(centroids_.cend(), centroids_);
+ }
+ bool Next() override {
+ if (Valid()) {
+ std::advance(iter_, 1);
+ }
+ return iter_ != centroids_.cend();
+ }
+
+ // The Prev function can only be called for item is not cend,
+ // because we must guarantee the iterator to be inside the valid range
before iteration.
+ bool Prev() override {
+ if (Valid() && iter_ != centroids_.cbegin()) {
+ std::advance(iter_, -1);
+ }
+ return Valid();
+ }
+ bool Valid() const override { return iter_ != centroids_.cend(); }
+ StatusOr<Centroid> GetCentroid() const override {
+ if (iter_ == centroids_.cend()) {
+ return {::Status::NotOK, "invalid iterator during decoding tdigest
centroid"};
+ }
+ return *iter_;
+ }
+
+ private:
+ std::vector<Centroid>::const_iterator iter_;
+ const std::vector<Centroid>& centroids_;
+ };
+
+ class ReverseIterator final : public BaseIterator {
Review Comment:
Could we have something like `template <bool Reverse> Iterator` instead of
virtual functions?
##########
src/types/redis_tdigest.h:
##########
@@ -33,6 +33,126 @@
#include "tdigest.h"
namespace redis {
+
+// TODO: It should be replaced by a iteration of the rocksdb iterator
+class DummyCentroids {
+ public:
+ class BaseIterator {
+ public:
+ virtual ~BaseIterator() = default;
+ virtual bool Next() = 0;
+ virtual bool Prev() = 0;
+ virtual bool Valid() const = 0;
+ virtual std::unique_ptr<BaseIterator> Clone() const = 0;
+ virtual StatusOr<Centroid> GetCentroid() const = 0;
+ };
+
+ DummyCentroids(const TDigestMetadata& meta_data, const
std::vector<Centroid>& centroids)
+ : meta_data_(meta_data), centroids_(centroids) {}
+ class Iterator : public BaseIterator {
+ public:
+ Iterator(std::vector<Centroid>::const_iterator&& iter, const
std::vector<Centroid>& centroids)
+ : iter_(iter), centroids_(centroids) {}
+ std::unique_ptr<BaseIterator> Clone() const override {
+ if (iter_ != centroids_.cend()) {
+ return std::make_unique<Iterator>(std::next(centroids_.cbegin(),
std::distance(centroids_.cbegin(), iter_)),
+ centroids_);
+ }
+ return std::make_unique<Iterator>(centroids_.cend(), centroids_);
+ }
+ bool Next() override {
+ if (Valid()) {
+ std::advance(iter_, 1);
+ }
+ return iter_ != centroids_.cend();
+ }
+
+ // The Prev function can only be called for item is not cend,
+ // because we must guarantee the iterator to be inside the valid range
before iteration.
+ bool Prev() override {
+ if (Valid() && iter_ != centroids_.cbegin()) {
+ std::advance(iter_, -1);
+ }
+ return Valid();
+ }
+ bool Valid() const override { return iter_ != centroids_.cend(); }
+ StatusOr<Centroid> GetCentroid() const override {
+ if (iter_ == centroids_.cend()) {
+ return {::Status::NotOK, "invalid iterator during decoding tdigest
centroid"};
+ }
+ return *iter_;
+ }
+
+ private:
+ std::vector<Centroid>::const_iterator iter_;
+ const std::vector<Centroid>& centroids_;
+ };
+
+ class ReverseIterator final : public BaseIterator {
Review Comment:
Could we have something like `template <bool Reverse> class Iterator`
instead of virtual functions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
