c++iteratorinput-iterator

Stateful C++ Input Iterators post increment problem


I was implementing an iterator that takes another float values producing input iterator and returns true if a rising was detected. So, the iterator works effectively as a Software-ADC (Analog-Digital-Converter).

I've minimized the actual code to the following:

#include <iterator>
template<typename It>
struct ADCFilter
{

  using iterator_tag = std::input_iterator_tag;

  ADCFilter(const float threshold, It it)
    : It_{it}, Threshold_{threshold}, LastAnalogVal_{0}
  {}

  bool operator*()
  {
    float analog_val = *It_;

    // rising edge
    if (analog_val >= Threshold_ && LastAnalogVal_< Threshold_)
    {
      LastAnalogVal_ = analog_val;
      return true;
    }

    // no rising edge
    LastAnalogVal_ = analog_val;
    return false;
  }

  ADCFilter operator++() { ++It_; return *this; }

  // Problem is here
  ADCFilter operator++(int) {auto self = *this; operator++(); return self; }
private:
  It    It_;
  float Threshold_;
  float LastAnalogVal_;
};

As you can see I need to cache the last analog value. And if somebody uses the iterator in such a way:

    std::vector<float> v = {...};
    auto f = ADCFilter(0.2f, v.begin());
    while(true) {
        std::cout << *f++; // <- post inc
    }

The cached value is never stored as it's only present in the returned copy. This problem doesn't occur though with pre increment because we are dereferencing the actual iterator and not a copy of it.

I could easily prohibit the usage of post increment operator by not implementing it, but according to https://en.cppreference.com/w/cpp/named_req/InputIterator it must be implemented for input iterators.

So, the question is how can I correctly implement a stateful input iterator that acts like a filter/mapper to another input iterator?


Solution

  • This can be done by reimplementing the operator so that its internal data holds just the bool value, instead of the floating point value from which the bool value is derived only when the iterator gets dereferenced.

    In other words, the dereferencing iterator should simply be:

    bool operator*() const // It should be const, by the way
    {
        return value;
    }
    
    // ...
    
    private:
    
        bool value;
    

    The constructor initializes the value to false. All the code that's currently in the dereferencing operator gets basically moved into the operator++(), and its end result is an updated value. It is not necessary to hold a copy of the real value getting iterated over, the operator++ can simply compare the current value referenced by the wrapped iterator with the new value after the wrapped iterator gets incremented, and update the value.

    The post-increment ++ operator remains unchanged.