If I were to port this over to generator, would you use it?

[JesseTG]

Found some code for generating icosahedral Goldberg polyhedra. If I ported it over, would you accept it?

[ilmola]

You may not be able to.

The problem is that generator uses pull (iterator like) API rather than push (callback) API. Implementing complex algorithms for pull style APIs without coroutine support is notoriously difficult.

I have been trying to change the API to use callbacks. This would significantly simplify the library implementation and make more complex things feasible.

// old pull style API
for (const MeshVertex& vertex : mesh.vertices()) {
    // use vertex
}

// new push style API
mesh.vertices([] (const MeshVertex& vertex) {
    // use vertex
});

Before (or if) this is done I think the Goldberg polyhedron is too much trouble to do.

[JesseTG]

What would such an API change entail, exactly? As much as I've extolled this library, I admittedly haven't looked at its implementation too much.

[ilmola]

It is quite trivial.

One simply needs copy paste to each primitive class an overload of form:

template <typename F>
void vertices(F fn) const { for (const auto& v : vertices()) fn(v); }

(And same for triangles and edges.)

I tried it and it works, but it makes the API inconsistent if new primitives are added that only support the new callback form. So it's not very good idea to merge it.

What I originally meant, was that if the code at https://github.com/PBAKER26/GoldbergPolyhedron is not written in the iterator (pull) form, to port it you need to do an inversion of control that is really difficult.

[JesseTG]

In such a case, why not just specify the "old" overload in terms of the "new" one (i.e. use the new-style one under the hood)?

[ilmola]

Which is exactly the kind of push to pull conversion that the whole new API addition idea was meant to avoid.

It seems that the best strategy for porting the code would be to first build internal cache in the primitive constructor and then just iterate the cache in the generator. This would make porting the code easy and does not need any API changes.

[JesseTG]

That could work. I think I'll go ahead and do that.