0

I have to create an iterator for an API that features old-style "write-out to pointer" accessors only. The API in question is OGR's; one of the classes in question is OGRLineString (For reference: Link). This class stores a number of points, which can be accessed using the following getter method:

 void OGRLineString::getPoint(int pos, OGRPoint *out)

In order to use the accessor one creates a new OGRPoint object and passes the pointer to it to the method, which writes the data to the allocated object. For example:

OGRPoint *p = new OGRPoint();
lineString->getPoint(0, p);

Now, I'd like to implement a (STL-like) iterator. Even if I place big warning signs everywhere stating that the supplied OGRPoints are non-modifiable (i.e., const), and won't update if another piece of code modifies the OGRLineString that is being iterated, I get a memory leak problem with OGRPoint const &operator*() const, because the API requires me to pass a custom-allocated OGRPoint instance, but the iterator would have to allocate one. Plus, the OGRPoints returned by the iterator shouldn't be deleted when the iterator itself is deleted. Additionally, the OGRLineString does not store actual instances of OGRPoint that are being copied for getPoint, but simple structs storing x/y/z coordinates; all required additional information (e.g., the spatial reference) is copied in the accessor. Thus, a simple #define private public hack wouldn't help.

Is there any sane/clean way to add an iterator without modifying the original source of OGRLineString? E.g., is there a way to add features to the original class, or modify it, like Ruby's "monkey patching" feature would do? Or watch the container's life time in order to clean up on the OGRPoint instances returned by the iterator?

Glorfindel
  • 21,988
  • 13
  • 81
  • 109
Technaton
  • 944
  • 4
  • 15
  • You should look at how `vector::operator[]` works, that will show you how to address this quirk. – Mooing Duck Jan 15 '15 at 18:09
  • What are you doing now that leads to a memory leak? Why is the iterator not managing the `OGRPoint`? What does your iterator do? – Mooing Duck Jan 15 '15 at 18:10
  • can OGRPoint be used with value-semantics? ie it has copy-constructor etc? In that case, do that, instead of 'new'-ing. if not, use smartpointers – sp2danny Jan 15 '15 at 18:30

2 Answers2

1
class this_is_my_iterator;

class OutputOGRPoint {
    explicit OutputOGRPoint(this_is_my_iterator* parent_)
        :parent(parent_), p(new OGRPoint()) 
    {}
    ~OutputOGRPoint();
    operator OGRPoint *() {return p;}

    OutputOGRPoint(OutputOGRPoint &&)=default;
    OutputOGRPoint&operator=(OutputOGRPoint &&)=default;
private:    
    this_is_my_iterator* parent;
    std::unique_ptr<OGRPoint> p;
};

class this_is_my_iterator {
    OutputOGRPoint operator*()(return OutputOGRPoint(this);}
private:
    friend OutputOGRPoint;
    void apply_operator_star_changes(OGRPoint *p);
};

inline OutputOGRPoint::~OutputOGRPoint()
{parent->apply_operator_star_changes(p);}

This "pseudo pointer" return type is used when you need code that manages the lifetime of a returned value. It can also be used to "return a mutable reference" for internal members that don't actually exist. vector<bool> uses bitfields internally instead of bool objects, but uses this same pattern to return a mutable reference from operator[].

Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
  • The output pointer concept was new to me, thanks alot! I learned quite a lot thanks to your and sp2danny's answer. I only later saw that I was also dependent on a Factory class of OGR (for WKT/WKB reading/writing), so the output pointer seemed more intrusive. Nevertheless, thank you for the suggestion that widened my C++ knowledge! – Technaton Jan 21 '15 at 14:48
1

This assumes that OGRPoint is copy-constructible. If not, use smart-pointers.

#include <iterator>

#include <ogr_geometry.h>

struct OGR_SimpleCurve_Points_Iterator : std::iterator< std::random_access_iterator_tag, const OGRPoint >
{
    OGR_SimpleCurve_Points_Iterator( OGRSimpleCurve* curve=nullptr, int index=0 )
        : curve(curve), index(index) {}

    OGR_SimpleCurve_Points_Iterator& operator++() { ++index; return *this; }
    OGR_SimpleCurve_Points_Iterator operator++(int) { OGR_SimpleCurve_Points_Iterator ret(*this); ++index; return ret; }
    OGR_SimpleCurve_Points_Iterator& operator--() { --index; return *this; }
    OGR_SimpleCurve_Points_Iterator operator--(int) { OGR_SimpleCurve_Points_Iterator ret(*this); --index; return ret; }

    OGR_SimpleCurve_Points_Iterator& operator+=(int n) { index+=n; return *this; }
    OGR_SimpleCurve_Points_Iterator& operator-=(int n) { index-=n; return *this; }

    OGR_SimpleCurve_Points_Iterator operator+(int n) { return OGR_SimpleCurve_Points_Iterator{curve,index+n}; }
    OGR_SimpleCurve_Points_Iterator operator-(int n) { return OGR_SimpleCurve_Points_Iterator{curve,index-n}; }

    int operator-(const OGR_SimpleCurve_Points_Iterator& other) { return index-other.index; }

    OGRPoint operator*() { OGRPoint p; curve->getPoint(index,&p); return p; }

    OGRPoint operator[](int ofs) { OGRPoint p; curve->getPoint(index+ofs,&p); return p; }

    bool operator == ( const OGR_SimpleCurve_Points_Iterator& other ) { return index==other.index; }
    bool operator != ( const OGR_SimpleCurve_Points_Iterator& other ) { return index!=other.index; }
    bool operator  > ( const OGR_SimpleCurve_Points_Iterator& other ) { return index >other.index; }
    bool operator >= ( const OGR_SimpleCurve_Points_Iterator& other ) { return index>=other.index; }
    bool operator  < ( const OGR_SimpleCurve_Points_Iterator& other ) { return index <other.index; }
    bool operator <= ( const OGR_SimpleCurve_Points_Iterator& other ) { return index<=other.index; }

private:
    OGRSimpleCurve* curve;
    int index;
};

OGR_SimpleCurve_Points_Iterator begin( OGRSimpleCurve* curve )
{
    return OGR_SimpleCurve_Points_Iterator{curve};
}

OGR_SimpleCurve_Points_Iterator end( OGRSimpleCurve* curve )
{
    return OGR_SimpleCurve_Points_Iterator{curve,curve->getNumPoints()};
}
sp2danny
  • 7,488
  • 3
  • 31
  • 53
  • Thank you very much! After quite some work (and reading up on C++-related fineties), this version was best suited to my needs. The copy-constructible vs. shared pointer vs. raw pointer remark finally made me actually understand the problem I was facing. Thanks again! – Technaton Jan 21 '15 at 14:45
  • 1
    This looks like a full and complete iterator, but without `operator->`, this iterator only fulfils the `output_iterator` concept. It's also missing all the typedefs. If one adds those, a default constructor, and `operator+(int,iterator)`, then it could receive the full `random_access_iterator_tag`. http://stackoverflow.com/a/8054856/845092 – Mooing Duck Jan 21 '15 at 17:32