Re: [boost] [Review Request] Inclusion of the Boost.Polygon Voronoi Library

Subject: Re: [boost] [Review Request] Inclusion of the Boost.Polygon Voronoi Library
From: Phil Endecott (spam_from_boost_dev_at_[hidden])
Date: 2012-04-13 14:51:56

Hi Andrii,

I've just tried this:

class delaunay_triangulation {

public:
  class Builder {
    delaunay_triangulation& dt;

  public:
    Builder(delaunay_triangulation& dt_): dt(dt_) {}

    void reserve(size_t num_sites) { dt.reserve(num_sites); }

    template <typename SEvent>
    void process_single_site(const SEvent& site) { dt.process_single_site(site); }

    template <typename SEvent>
    std::pair<void*,void*> insert_new_edge(const SEvent& site1, const SEvent& site2) { return dt.insert_new_edge(site1,site2); }

    template <typename SEvent, typename CEvent>
    std::pair<void*, void*> insert_new_edge(const SEvent& site1, const SEvent& site3, const CEvent& circle, void* data12, void* da$
      return dt.insert_new_edge(site1, site3, circle, data12, data23);
    }

    void build() { dt.build(); }
  };

  delaunay_triangulation():
    builder_(*this)
  {}

  typedef std::pair<int,int> point_t;
  typedef std::pair<point_t,point_t> edge_t;
  typedef std::vector<edge_t> edges_t;
  edges_t edges;

  Builder* builder() { return &builder_; }  // Why not a reference?

  void reserve(size_t /*num_sites*/) {}

  template <typename SEvent>
  void process_single_site(const SEvent& /*site*/) {}

  template <typename SEvent>
  std::pair<void*, void*> insert_new_edge(const SEvent& site1, const SEvent& site2)
  {
    edge_t e(point_t(site1.x0(),site1.y0()),
             point_t(site2.x0(),site2.y0()));
    edges.push_back(e);
    return std::pair<void*,void*>(NULL,NULL);
  }

  template <typename SEvent, typename CEvent>
  std::pair<void*, void*> insert_new_edge(const SEvent& site1, const SEvent& site3,
                                          const CEvent& /*circle*/, void* /*data12*/, void* /*data23*/) {
    return insert_new_edge(site1,site3);
  }

  void build() {}

private:
  Builder builder_;

};

It does seem to create a triangulation as you can see here:

  http://chezphil.org/tmp/delaunay.png

A couple of observations:

- Again you're using void* in here, which doesn't look great.  Is this just 
to break circular references, or something?  Why can't you use the actual 
types?

- Your "builder" class, which just forwards to the other class, doesn't 
seem very useful to me.  I can see the idea - you're breaking the 
"lifecycle" of the object into a "build" phase and then a "use" phase - 
but I think there are probably better ways to do it if it's worth doing 
at all.

Anyway, I think you potentially have something very useful here.  Thanks!

Regards,  Phil.